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: Mimi Zohar <zohar@us.ibm.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Signed-off-by: James Morris <jmorris@namei.org>
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 42706b5..e3c16a2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -97,6 +97,7 @@
/* 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 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;
};
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index 750db3c..1f035e8 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -126,6 +126,7 @@
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 @@
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 @@
mutex_init(&iint->mutex);
iint->readcount = 0;
iint->writecount = 0;
+ iint->opencount = 0;
kref_set(&iint->refcount, 1);
}
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 871e356..f4e7266 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -66,6 +66,19 @@
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 @@
pr_info("%s dentry_open failed\n", filename);
return rc;
}
+ iint->opencount++;
iint->readcount++;
rc = ima_collect_measurement(iint, file);
@@ -159,6 +173,7 @@
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 @@
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 @@
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