diff options
author | Mimi Zohar <zohar@linux.vnet.ibm.com> | 2009-02-04 09:07:02 -0500 |
---|---|---|
committer | James Morris <jmorris@namei.org> | 2009-02-05 17:05:33 -0500 |
commit | 1df9f0a73178718969ae47d813b8e7aab2cf073c (patch) | |
tree | 6bd3d8838858f0e93acd8f7969b7d0e5ce2bfb08 | |
parent | f4bd857bc8ed997c25ec06b56ef8064aafa6d4f3 (diff) |
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>
-rw-r--r-- | include/linux/ima.h | 6 | ||||
-rw-r--r-- | ipc/shm.c | 3 | ||||
-rw-r--r-- | mm/shmem.c | 2 | ||||
-rw-r--r-- | security/integrity/ima/ima.h | 2 | ||||
-rw-r--r-- | security/integrity/ima/ima_iint.c | 17 | ||||
-rw-r--r-- | security/integrity/ima/ima_main.c | 42 |
6 files changed, 72 insertions, 0 deletions
diff --git a/include/linux/ima.h b/include/linux/ima.h index dcc3664feee8..6db30a328d98 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h | |||
@@ -19,6 +19,7 @@ extern void ima_inode_free(struct inode *inode); | |||
19 | extern int ima_path_check(struct path *path, int mask); | 19 | extern int ima_path_check(struct path *path, int mask); |
20 | extern void ima_file_free(struct file *file); | 20 | extern void ima_file_free(struct file *file); |
21 | extern int ima_file_mmap(struct file *file, unsigned long prot); | 21 | extern int ima_file_mmap(struct file *file, unsigned long prot); |
22 | extern void ima_shm_check(struct file *file); | ||
22 | 23 | ||
23 | #else | 24 | #else |
24 | static inline int ima_bprm_check(struct linux_binprm *bprm) | 25 | 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) | |||
50 | { | 51 | { |
51 | return 0; | 52 | return 0; |
52 | } | 53 | } |
54 | |||
55 | static inline void ima_shm_check(struct file *file) | ||
56 | { | ||
57 | return; | ||
58 | } | ||
53 | #endif /* CONFIG_IMA_H */ | 59 | #endif /* CONFIG_IMA_H */ |
54 | #endif /* _LINUX_IMA_H */ | 60 | #endif /* _LINUX_IMA_H */ |
@@ -39,6 +39,7 @@ | |||
39 | #include <linux/nsproxy.h> | 39 | #include <linux/nsproxy.h> |
40 | #include <linux/mount.h> | 40 | #include <linux/mount.h> |
41 | #include <linux/ipc_namespace.h> | 41 | #include <linux/ipc_namespace.h> |
42 | #include <linux/ima.h> | ||
42 | 43 | ||
43 | #include <asm/uaccess.h> | 44 | #include <asm/uaccess.h> |
44 | 45 | ||
@@ -381,6 +382,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params) | |||
381 | error = PTR_ERR(file); | 382 | error = PTR_ERR(file); |
382 | if (IS_ERR(file)) | 383 | if (IS_ERR(file)) |
383 | goto no_file; | 384 | goto no_file; |
385 | ima_shm_check(file); | ||
384 | 386 | ||
385 | id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni); | 387 | id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni); |
386 | if (id < 0) { | 388 | if (id < 0) { |
@@ -888,6 +890,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg, ulong *raddr) | |||
888 | file = alloc_file(path.mnt, path.dentry, f_mode, &shm_file_operations); | 890 | file = alloc_file(path.mnt, path.dentry, f_mode, &shm_file_operations); |
889 | if (!file) | 891 | if (!file) |
890 | goto out_free; | 892 | goto out_free; |
893 | ima_shm_check(file); | ||
891 | 894 | ||
892 | file->private_data = sfd; | 895 | file->private_data = sfd; |
893 | file->f_mapping = shp->shm_file->f_mapping; | 896 | file->f_mapping = shp->shm_file->f_mapping; |
diff --git a/mm/shmem.c b/mm/shmem.c index f1b0d4871f3a..dd5588f5d939 100644 --- a/mm/shmem.c +++ b/mm/shmem.c | |||
@@ -51,6 +51,7 @@ | |||
51 | #include <linux/highmem.h> | 51 | #include <linux/highmem.h> |
52 | #include <linux/seq_file.h> | 52 | #include <linux/seq_file.h> |
53 | #include <linux/magic.h> | 53 | #include <linux/magic.h> |
54 | #include <linux/ima.h> | ||
54 | 55 | ||
55 | #include <asm/uaccess.h> | 56 | #include <asm/uaccess.h> |
56 | #include <asm/div64.h> | 57 | #include <asm/div64.h> |
@@ -2600,6 +2601,7 @@ int shmem_zero_setup(struct vm_area_struct *vma) | |||
2600 | if (IS_ERR(file)) | 2601 | if (IS_ERR(file)) |
2601 | return PTR_ERR(file); | 2602 | return PTR_ERR(file); |
2602 | 2603 | ||
2604 | ima_shm_check(file); | ||
2603 | if (vma->vm_file) | 2605 | if (vma->vm_file) |
2604 | fput(vma->vm_file); | 2606 | fput(vma->vm_file); |
2605 | vma->vm_file = file; | 2607 | vma->vm_file = file; |
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 42706b554921..e3c16a21a38e 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h | |||
@@ -97,6 +97,7 @@ static inline unsigned long ima_hash_key(u8 *digest) | |||
97 | 97 | ||
98 | /* iint cache flags */ | 98 | /* iint cache flags */ |
99 | #define IMA_MEASURED 1 | 99 | #define IMA_MEASURED 1 |
100 | #define IMA_IINT_DUMP_STACK 512 | ||
100 | 101 | ||
101 | /* integrity data associated with an inode */ | 102 | /* integrity data associated with an inode */ |
102 | struct ima_iint_cache { | 103 | struct ima_iint_cache { |
@@ -106,6 +107,7 @@ struct ima_iint_cache { | |||
106 | struct mutex mutex; /* protects: version, flags, digest */ | 107 | struct mutex mutex; /* protects: version, flags, digest */ |
107 | long readcount; /* measured files readcount */ | 108 | long readcount; /* measured files readcount */ |
108 | long writecount; /* measured files writecount */ | 109 | long writecount; /* measured files writecount */ |
110 | long opencount; /* opens reference count */ | ||
109 | struct kref refcount; /* ima_iint_cache reference count */ | 111 | struct kref refcount; /* ima_iint_cache reference count */ |
110 | struct rcu_head rcu; | 112 | struct rcu_head rcu; |
111 | }; | 113 | }; |
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c index 750db3c993a7..1f035e8d29c7 100644 --- a/security/integrity/ima/ima_iint.c +++ b/security/integrity/ima/ima_iint.c | |||
@@ -126,6 +126,7 @@ struct ima_iint_cache *ima_iint_find_insert_get(struct inode *inode) | |||
126 | 126 | ||
127 | return iint; | 127 | return iint; |
128 | } | 128 | } |
129 | EXPORT_SYMBOL_GPL(ima_iint_find_insert_get); | ||
129 | 130 | ||
130 | /* iint_free - called when the iint refcount goes to zero */ | 131 | /* iint_free - called when the iint refcount goes to zero */ |
131 | void iint_free(struct kref *kref) | 132 | void iint_free(struct kref *kref) |
@@ -134,6 +135,21 @@ void iint_free(struct kref *kref) | |||
134 | refcount); | 135 | refcount); |
135 | iint->version = 0; | 136 | iint->version = 0; |
136 | iint->flags = 0UL; | 137 | iint->flags = 0UL; |
138 | if (iint->readcount != 0) { | ||
139 | printk(KERN_INFO "%s: readcount: %ld\n", __FUNCTION__, | ||
140 | iint->readcount); | ||
141 | iint->readcount = 0; | ||
142 | } | ||
143 | if (iint->writecount != 0) { | ||
144 | printk(KERN_INFO "%s: writecount: %ld\n", __FUNCTION__, | ||
145 | iint->writecount); | ||
146 | iint->writecount = 0; | ||
147 | } | ||
148 | if (iint->opencount != 0) { | ||
149 | printk(KERN_INFO "%s: opencount: %ld\n", __FUNCTION__, | ||
150 | iint->opencount); | ||
151 | iint->opencount = 0; | ||
152 | } | ||
137 | kref_set(&iint->refcount, 1); | 153 | kref_set(&iint->refcount, 1); |
138 | kmem_cache_free(iint_cache, iint); | 154 | kmem_cache_free(iint_cache, iint); |
139 | } | 155 | } |
@@ -174,6 +190,7 @@ static void init_once(void *foo) | |||
174 | mutex_init(&iint->mutex); | 190 | mutex_init(&iint->mutex); |
175 | iint->readcount = 0; | 191 | iint->readcount = 0; |
176 | iint->writecount = 0; | 192 | iint->writecount = 0; |
193 | iint->opencount = 0; | ||
177 | kref_set(&iint->refcount, 1); | 194 | kref_set(&iint->refcount, 1); |
178 | } | 195 | } |
179 | 196 | ||
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 871e356e8d6c..f4e7266f5aee 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c | |||
@@ -66,6 +66,19 @@ void ima_file_free(struct file *file) | |||
66 | return; | 66 | return; |
67 | 67 | ||
68 | mutex_lock(&iint->mutex); | 68 | mutex_lock(&iint->mutex); |
69 | if (iint->opencount <= 0) { | ||
70 | printk(KERN_INFO | ||
71 | "%s: %s open/free imbalance (r:%ld w:%ld o:%ld f:%ld)\n", | ||
72 | __FUNCTION__, file->f_dentry->d_name.name, | ||
73 | iint->readcount, iint->writecount, | ||
74 | iint->opencount, atomic_long_read(&file->f_count)); | ||
75 | if (!(iint->flags & IMA_IINT_DUMP_STACK)) { | ||
76 | dump_stack(); | ||
77 | iint->flags |= IMA_IINT_DUMP_STACK; | ||
78 | } | ||
79 | } | ||
80 | iint->opencount--; | ||
81 | |||
69 | if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) | 82 | if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) |
70 | iint->readcount--; | 83 | iint->readcount--; |
71 | 84 | ||
@@ -119,6 +132,7 @@ static int get_path_measurement(struct ima_iint_cache *iint, struct file *file, | |||
119 | pr_info("%s dentry_open failed\n", filename); | 132 | pr_info("%s dentry_open failed\n", filename); |
120 | return rc; | 133 | return rc; |
121 | } | 134 | } |
135 | iint->opencount++; | ||
122 | iint->readcount++; | 136 | iint->readcount++; |
123 | 137 | ||
124 | rc = ima_collect_measurement(iint, file); | 138 | rc = ima_collect_measurement(iint, file); |
@@ -159,6 +173,7 @@ int ima_path_check(struct path *path, int mask) | |||
159 | return 0; | 173 | return 0; |
160 | 174 | ||
161 | mutex_lock(&iint->mutex); | 175 | mutex_lock(&iint->mutex); |
176 | iint->opencount++; | ||
162 | if ((mask & MAY_WRITE) || (mask == 0)) | 177 | if ((mask & MAY_WRITE) || (mask == 0)) |
163 | iint->writecount++; | 178 | iint->writecount++; |
164 | else if (mask & (MAY_READ | MAY_EXEC)) | 179 | else if (mask & (MAY_READ | MAY_EXEC)) |
@@ -219,6 +234,21 @@ out: | |||
219 | return rc; | 234 | return rc; |
220 | } | 235 | } |
221 | 236 | ||
237 | static void opencount_get(struct file *file) | ||
238 | { | ||
239 | struct inode *inode = file->f_dentry->d_inode; | ||
240 | struct ima_iint_cache *iint; | ||
241 | |||
242 | if (!ima_initialized || !S_ISREG(inode->i_mode)) | ||
243 | return; | ||
244 | iint = ima_iint_find_insert_get(inode); | ||
245 | if (!iint) | ||
246 | return; | ||
247 | mutex_lock(&iint->mutex); | ||
248 | iint->opencount++; | ||
249 | mutex_unlock(&iint->mutex); | ||
250 | } | ||
251 | |||
222 | /** | 252 | /** |
223 | * ima_file_mmap - based on policy, collect/store measurement. | 253 | * ima_file_mmap - based on policy, collect/store measurement. |
224 | * @file: pointer to the file to be measured (May be NULL) | 254 | * @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) | |||
242 | return 0; | 272 | return 0; |
243 | } | 273 | } |
244 | 274 | ||
275 | /* | ||
276 | * ima_shm_check - IPC shm and shmat create/fput a file | ||
277 | * | ||
278 | * Maintain the opencount for these files to prevent unnecessary | ||
279 | * imbalance messages. | ||
280 | */ | ||
281 | void ima_shm_check(struct file *file) | ||
282 | { | ||
283 | opencount_get(file); | ||
284 | return; | ||
285 | } | ||
286 | |||
245 | /** | 287 | /** |
246 | * ima_bprm_check - based on policy, collect/store measurement. | 288 | * ima_bprm_check - based on policy, collect/store measurement. |
247 | * @bprm: contains the linux_binprm structure | 289 | * @bprm: contains the linux_binprm structure |