Commit 1df9f0a7 authored by Mimi Zohar's avatar Mimi Zohar Committed by James Morris

Integrity: IMA file free imbalance

The number of calls to ima_path_check()/ima_file_free()
should be balanced.  An extra call to fput(), indicates
the file could have been accessed without first being
measured.

Although f_count is incremented/decremented in places other
than fget/fput, like fget_light/fput_light and get_file, the
current task must already hold a file refcnt.  The call to
__fput() is delayed until the refcnt becomes 0, resulting
in ima_file_free() flagging any changes.

- add hook to increment opencount for IPC shared memory(SYSV),
  shmat files, and /dev/zero
- moved NULL iint test in opencount_get()
Signed-off-by: default avatarMimi Zohar <zohar@us.ibm.com>
Acked-by: default avatarSerge Hallyn <serue@us.ibm.com>
Signed-off-by: default avatarJames Morris <jmorris@namei.org>
parent f4bd857b
......@@ -19,6 +19,7 @@ extern void ima_inode_free(struct inode *inode);
extern int ima_path_check(struct path *path, int mask);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
extern void ima_shm_check(struct file *file);
#else
static inline int ima_bprm_check(struct linux_binprm *bprm)
......@@ -50,5 +51,10 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
{
return 0;
}
static inline void ima_shm_check(struct file *file)
{
return;
}
#endif /* CONFIG_IMA_H */
#endif /* _LINUX_IMA_H */
......@@ -39,6 +39,7 @@
#include <linux/nsproxy.h>
#include <linux/mount.h>
#include <linux/ipc_namespace.h>
#include <linux/ima.h>
#include <asm/uaccess.h>
......@@ -381,6 +382,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
error = PTR_ERR(file);
if (IS_ERR(file))
goto no_file;
ima_shm_check(file);
id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
if (id < 0) {
......@@ -888,6 +890,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr)
file = alloc_file(path.mnt, path.dentry, f_mode, &shm_file_operations);
if (!file)
goto out_free;
ima_shm_check(file);
file->private_data = sfd;
file->f_mapping = shp->shm_file->f_mapping;
......
......@@ -51,6 +51,7 @@
#include <linux/highmem.h>
#include <linux/seq_file.h>
#include <linux/magic.h>
#include <linux/ima.h>
#include <asm/uaccess.h>
#include <asm/div64.h>
......@@ -2600,6 +2601,7 @@ int shmem_zero_setup(struct vm_area_struct *vma)
if (IS_ERR(file))
return PTR_ERR(file);
ima_shm_check(file);
if (vma->vm_file)
fput(vma->vm_file);
vma->vm_file = file;
......
......@@ -97,6 +97,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
/* iint cache flags */
#define IMA_MEASURED 1
#define IMA_IINT_DUMP_STACK 512
/* integrity data associated with an inode */
struct ima_iint_cache {
......@@ -106,6 +107,7 @@ struct ima_iint_cache {
struct mutex mutex; /* protects: version, flags, digest */
long readcount; /* measured files readcount */
long writecount; /* measured files writecount */
long opencount; /* opens reference count */
struct kref refcount; /* ima_iint_cache reference count */
struct rcu_head rcu;
};
......
......@@ -126,6 +126,7 @@ struct ima_iint_cache *ima_iint_find_insert_get(struct inode *inode)
return iint;
}
EXPORT_SYMBOL_GPL(ima_iint_find_insert_get);
/* iint_free - called when the iint refcount goes to zero */
void iint_free(struct kref *kref)
......@@ -134,6 +135,21 @@ void iint_free(struct kref *kref)
refcount);
iint->version = 0;
iint->flags = 0UL;
if (iint->readcount != 0) {
printk(KERN_INFO "%s: readcount: %ld\n", __FUNCTION__,
iint->readcount);
iint->readcount = 0;
}
if (iint->writecount != 0) {
printk(KERN_INFO "%s: writecount: %ld\n", __FUNCTION__,
iint->writecount);
iint->writecount = 0;
}
if (iint->opencount != 0) {
printk(KERN_INFO "%s: opencount: %ld\n", __FUNCTION__,
iint->opencount);
iint->opencount = 0;
}
kref_set(&iint->refcount, 1);
kmem_cache_free(iint_cache, iint);
}
......@@ -174,6 +190,7 @@ static void init_once(void *foo)
mutex_init(&iint->mutex);
iint->readcount = 0;
iint->writecount = 0;
iint->opencount = 0;
kref_set(&iint->refcount, 1);
}
......
......@@ -66,6 +66,19 @@ void ima_file_free(struct file *file)
return;
mutex_lock(&iint->mutex);
if (iint->opencount <= 0) {
printk(KERN_INFO
"%s: %s open/free imbalance (r:%ld w:%ld o:%ld f:%ld)\n",
__FUNCTION__, file->f_dentry->d_name.name,
iint->readcount, iint->writecount,
iint->opencount, atomic_long_read(&file->f_count));
if (!(iint->flags & IMA_IINT_DUMP_STACK)) {
dump_stack();
iint->flags |= IMA_IINT_DUMP_STACK;
}
}
iint->opencount--;
if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
iint->readcount--;
......@@ -119,6 +132,7 @@ static int get_path_measurement(struct ima_iint_cache *iint, struct file *file,
pr_info("%s dentry_open failed\n", filename);
return rc;
}
iint->opencount++;
iint->readcount++;
rc = ima_collect_measurement(iint, file);
......@@ -159,6 +173,7 @@ int ima_path_check(struct path *path, int mask)
return 0;
mutex_lock(&iint->mutex);
iint->opencount++;
if ((mask & MAY_WRITE) || (mask == 0))
iint->writecount++;
else if (mask & (MAY_READ | MAY_EXEC))
......@@ -219,6 +234,21 @@ static int process_measurement(struct file *file, const unsigned char *filename,
return rc;
}
static void opencount_get(struct file *file)
{
struct inode *inode = file->f_dentry->d_inode;
struct ima_iint_cache *iint;
if (!ima_initialized || !S_ISREG(inode->i_mode))
return;
iint = ima_iint_find_insert_get(inode);
if (!iint)
return;
mutex_lock(&iint->mutex);
iint->opencount++;
mutex_unlock(&iint->mutex);
}
/**
* ima_file_mmap - based on policy, collect/store measurement.
* @file: pointer to the file to be measured (May be NULL)
......@@ -242,6 +272,18 @@ int ima_file_mmap(struct file *file, unsigned long prot)
return 0;
}
/*
* ima_shm_check - IPC shm and shmat create/fput a file
*
* Maintain the opencount for these files to prevent unnecessary
* imbalance messages.
*/
void ima_shm_check(struct file *file)
{
opencount_get(file);
return;
}
/**
* ima_bprm_check - based on policy, collect/store measurement.
* @bprm: contains the linux_binprm structure
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment