diff options
author | Mimi Zohar <zohar@linux.vnet.ibm.com> | 2010-01-20 15:35:41 -0500 |
---|---|---|
committer | Al Viro <viro@zeniv.linux.org.uk> | 2010-02-07 03:06:22 -0500 |
commit | 8eb988c70e7709b7bd1a69f0ec53d19ac20dea84 (patch) | |
tree | 6d0283a9fbca5cc104f591b9cc628edf39bc0b05 | |
parent | 1e41568d7378d1ba8c64ba137b9ddd00b59f893a (diff) |
fix ima breakage
The "Untangling ima mess, part 2 with counters" patch messed
up the counters. Based on conversations with Al Viro, this patch
streamlines ima_path_check() by removing the counter maintaince.
The counters are now updated independently, from measuring the file,
in __dentry_open() and alloc_file() by calling ima_counts_get().
ima_path_check() is called from nfsd and do_filp_open().
It also did not measure all files that should have been measured.
Reason: ima_path_check() got bogus value passed as mask.
[AV: mea culpa]
[AV: add missing nfsd bits]
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
-rw-r--r-- | fs/namei.c | 6 | ||||
-rw-r--r-- | fs/nfsd/vfs.c | 3 | ||||
-rw-r--r-- | include/linux/ima.h | 4 | ||||
-rw-r--r-- | security/integrity/ima/ima_main.c | 236 |
4 files changed, 97 insertions, 152 deletions
diff --git a/fs/namei.c b/fs/namei.c index 94a5e60779f9..cd77b6375efd 100644 --- a/fs/namei.c +++ b/fs/namei.c | |||
@@ -1736,8 +1736,7 @@ do_last: | |||
1736 | if (nd.root.mnt) | 1736 | if (nd.root.mnt) |
1737 | path_put(&nd.root); | 1737 | path_put(&nd.root); |
1738 | if (!IS_ERR(filp)) { | 1738 | if (!IS_ERR(filp)) { |
1739 | error = ima_path_check(&filp->f_path, filp->f_mode & | 1739 | error = ima_path_check(filp, acc_mode); |
1740 | (MAY_READ | MAY_WRITE | MAY_EXEC)); | ||
1741 | if (error) { | 1740 | if (error) { |
1742 | fput(filp); | 1741 | fput(filp); |
1743 | filp = ERR_PTR(error); | 1742 | filp = ERR_PTR(error); |
@@ -1797,8 +1796,7 @@ ok: | |||
1797 | } | 1796 | } |
1798 | filp = nameidata_to_filp(&nd); | 1797 | filp = nameidata_to_filp(&nd); |
1799 | if (!IS_ERR(filp)) { | 1798 | if (!IS_ERR(filp)) { |
1800 | error = ima_path_check(&filp->f_path, filp->f_mode & | 1799 | error = ima_path_check(filp, acc_mode); |
1801 | (MAY_READ | MAY_WRITE | MAY_EXEC)); | ||
1802 | if (error) { | 1800 | if (error) { |
1803 | fput(filp); | 1801 | fput(filp); |
1804 | filp = ERR_PTR(error); | 1802 | filp = ERR_PTR(error); |
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 325959e264ce..32477e3a645c 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c | |||
@@ -752,8 +752,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, int type, | |||
752 | flags, current_cred()); | 752 | flags, current_cred()); |
753 | if (IS_ERR(*filp)) | 753 | if (IS_ERR(*filp)) |
754 | host_err = PTR_ERR(*filp); | 754 | host_err = PTR_ERR(*filp); |
755 | host_err = ima_path_check(&(*filp)->f_path, | 755 | host_err = ima_path_check(*filp, access); |
756 | access & (MAY_READ | MAY_WRITE | MAY_EXEC)); | ||
757 | out_nfserr: | 756 | out_nfserr: |
758 | err = nfserrno(host_err); | 757 | err = nfserrno(host_err); |
759 | out: | 758 | out: |
diff --git a/include/linux/ima.h b/include/linux/ima.h index 99dc6d5cf7e5..aa55a8f1f5b9 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h | |||
@@ -17,7 +17,7 @@ struct linux_binprm; | |||
17 | extern int ima_bprm_check(struct linux_binprm *bprm); | 17 | extern int ima_bprm_check(struct linux_binprm *bprm); |
18 | extern int ima_inode_alloc(struct inode *inode); | 18 | extern int ima_inode_alloc(struct inode *inode); |
19 | extern void ima_inode_free(struct inode *inode); | 19 | extern void ima_inode_free(struct inode *inode); |
20 | extern int ima_path_check(struct path *path, int mask); | 20 | extern int ima_path_check(struct file *file, int mask); |
21 | extern void ima_file_free(struct file *file); | 21 | extern void ima_file_free(struct file *file); |
22 | extern int ima_file_mmap(struct file *file, unsigned long prot); | 22 | extern int ima_file_mmap(struct file *file, unsigned long prot); |
23 | extern void ima_counts_get(struct file *file); | 23 | extern void ima_counts_get(struct file *file); |
@@ -38,7 +38,7 @@ static inline void ima_inode_free(struct inode *inode) | |||
38 | return; | 38 | return; |
39 | } | 39 | } |
40 | 40 | ||
41 | static inline int ima_path_check(struct path *path, int mask) | 41 | static inline int ima_path_check(struct file *file, int mask) |
42 | { | 42 | { |
43 | return 0; | 43 | return 0; |
44 | } | 44 | } |
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index a89f44d5e030..75aee18f6163 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c | |||
@@ -84,6 +84,36 @@ out: | |||
84 | return found; | 84 | return found; |
85 | } | 85 | } |
86 | 86 | ||
87 | /* ima_read_write_check - reflect possible reading/writing errors in the PCR. | ||
88 | * | ||
89 | * When opening a file for read, if the file is already open for write, | ||
90 | * the file could change, resulting in a file measurement error. | ||
91 | * | ||
92 | * Opening a file for write, if the file is already open for read, results | ||
93 | * in a time of measure, time of use (ToMToU) error. | ||
94 | * | ||
95 | * In either case invalidate the PCR. | ||
96 | */ | ||
97 | enum iint_pcr_error { TOMTOU, OPEN_WRITERS }; | ||
98 | static void ima_read_write_check(enum iint_pcr_error error, | ||
99 | struct ima_iint_cache *iint, | ||
100 | struct inode *inode, | ||
101 | const unsigned char *filename) | ||
102 | { | ||
103 | switch (error) { | ||
104 | case TOMTOU: | ||
105 | if (iint->readcount > 0) | ||
106 | ima_add_violation(inode, filename, "invalid_pcr", | ||
107 | "ToMToU"); | ||
108 | break; | ||
109 | case OPEN_WRITERS: | ||
110 | if (iint->writecount > 0) | ||
111 | ima_add_violation(inode, filename, "invalid_pcr", | ||
112 | "open_writers"); | ||
113 | break; | ||
114 | } | ||
115 | } | ||
116 | |||
87 | /* | 117 | /* |
88 | * Update the counts given an fmode_t | 118 | * Update the counts given an fmode_t |
89 | */ | 119 | */ |
@@ -99,6 +129,47 @@ static void ima_inc_counts(struct ima_iint_cache *iint, fmode_t mode) | |||
99 | } | 129 | } |
100 | 130 | ||
101 | /* | 131 | /* |
132 | * ima_counts_get - increment file counts | ||
133 | * | ||
134 | * Maintain read/write counters for all files, but only | ||
135 | * invalidate the PCR for measured files: | ||
136 | * - Opening a file for write when already open for read, | ||
137 | * results in a time of measure, time of use (ToMToU) error. | ||
138 | * - Opening a file for read when already open for write, | ||
139 | * could result in a file measurement error. | ||
140 | * | ||
141 | */ | ||
142 | void ima_counts_get(struct file *file) | ||
143 | { | ||
144 | struct dentry *dentry = file->f_path.dentry; | ||
145 | struct inode *inode = dentry->d_inode; | ||
146 | fmode_t mode = file->f_mode; | ||
147 | struct ima_iint_cache *iint; | ||
148 | int rc; | ||
149 | |||
150 | if (!ima_initialized || !S_ISREG(inode->i_mode)) | ||
151 | return; | ||
152 | iint = ima_iint_find_get(inode); | ||
153 | if (!iint) | ||
154 | return; | ||
155 | mutex_lock(&iint->mutex); | ||
156 | rc = ima_must_measure(iint, inode, MAY_READ, PATH_CHECK); | ||
157 | if (rc < 0) | ||
158 | goto out; | ||
159 | |||
160 | if (mode & FMODE_WRITE) { | ||
161 | ima_read_write_check(TOMTOU, iint, inode, dentry->d_name.name); | ||
162 | goto out; | ||
163 | } | ||
164 | ima_read_write_check(OPEN_WRITERS, iint, inode, dentry->d_name.name); | ||
165 | out: | ||
166 | ima_inc_counts(iint, file->f_mode); | ||
167 | mutex_unlock(&iint->mutex); | ||
168 | |||
169 | kref_put(&iint->refcount, iint_free); | ||
170 | } | ||
171 | |||
172 | /* | ||
102 | * Decrement ima counts | 173 | * Decrement ima counts |
103 | */ | 174 | */ |
104 | static void ima_dec_counts(struct ima_iint_cache *iint, struct inode *inode, | 175 | static void ima_dec_counts(struct ima_iint_cache *iint, struct inode *inode, |
@@ -153,123 +224,6 @@ void ima_file_free(struct file *file) | |||
153 | kref_put(&iint->refcount, iint_free); | 224 | kref_put(&iint->refcount, iint_free); |
154 | } | 225 | } |
155 | 226 | ||
156 | /* ima_read_write_check - reflect possible reading/writing errors in the PCR. | ||
157 | * | ||
158 | * When opening a file for read, if the file is already open for write, | ||
159 | * the file could change, resulting in a file measurement error. | ||
160 | * | ||
161 | * Opening a file for write, if the file is already open for read, results | ||
162 | * in a time of measure, time of use (ToMToU) error. | ||
163 | * | ||
164 | * In either case invalidate the PCR. | ||
165 | */ | ||
166 | enum iint_pcr_error { TOMTOU, OPEN_WRITERS }; | ||
167 | static void ima_read_write_check(enum iint_pcr_error error, | ||
168 | struct ima_iint_cache *iint, | ||
169 | struct inode *inode, | ||
170 | const unsigned char *filename) | ||
171 | { | ||
172 | switch (error) { | ||
173 | case TOMTOU: | ||
174 | if (iint->readcount > 0) | ||
175 | ima_add_violation(inode, filename, "invalid_pcr", | ||
176 | "ToMToU"); | ||
177 | break; | ||
178 | case OPEN_WRITERS: | ||
179 | if (iint->writecount > 0) | ||
180 | ima_add_violation(inode, filename, "invalid_pcr", | ||
181 | "open_writers"); | ||
182 | break; | ||
183 | } | ||
184 | } | ||
185 | |||
186 | static int get_path_measurement(struct ima_iint_cache *iint, struct file *file, | ||
187 | const unsigned char *filename) | ||
188 | { | ||
189 | int rc = 0; | ||
190 | |||
191 | ima_inc_counts(iint, file->f_mode); | ||
192 | |||
193 | rc = ima_collect_measurement(iint, file); | ||
194 | if (!rc) | ||
195 | ima_store_measurement(iint, file, filename); | ||
196 | return rc; | ||
197 | } | ||
198 | |||
199 | /** | ||
200 | * ima_path_check - based on policy, collect/store measurement. | ||
201 | * @path: contains a pointer to the path to be measured | ||
202 | * @mask: contains MAY_READ, MAY_WRITE or MAY_EXECUTE | ||
203 | * | ||
204 | * Measure the file being open for readonly, based on the | ||
205 | * ima_must_measure() policy decision. | ||
206 | * | ||
207 | * Keep read/write counters for all files, but only | ||
208 | * invalidate the PCR for measured files: | ||
209 | * - Opening a file for write when already open for read, | ||
210 | * results in a time of measure, time of use (ToMToU) error. | ||
211 | * - Opening a file for read when already open for write, | ||
212 | * could result in a file measurement error. | ||
213 | * | ||
214 | * Always return 0 and audit dentry_open failures. | ||
215 | * (Return code will be based upon measurement appraisal.) | ||
216 | */ | ||
217 | int ima_path_check(struct path *path, int mask) | ||
218 | { | ||
219 | struct inode *inode = path->dentry->d_inode; | ||
220 | struct ima_iint_cache *iint; | ||
221 | struct file *file = NULL; | ||
222 | int rc; | ||
223 | |||
224 | if (!ima_initialized || !S_ISREG(inode->i_mode)) | ||
225 | return 0; | ||
226 | iint = ima_iint_find_get(inode); | ||
227 | if (!iint) | ||
228 | return 0; | ||
229 | |||
230 | mutex_lock(&iint->mutex); | ||
231 | |||
232 | rc = ima_must_measure(iint, inode, MAY_READ, PATH_CHECK); | ||
233 | if (rc < 0) | ||
234 | goto out; | ||
235 | |||
236 | if ((mask & MAY_WRITE) || (mask == 0)) | ||
237 | ima_read_write_check(TOMTOU, iint, inode, | ||
238 | path->dentry->d_name.name); | ||
239 | |||
240 | if ((mask & (MAY_WRITE | MAY_READ | MAY_EXEC)) != MAY_READ) | ||
241 | goto out; | ||
242 | |||
243 | ima_read_write_check(OPEN_WRITERS, iint, inode, | ||
244 | path->dentry->d_name.name); | ||
245 | if (!(iint->flags & IMA_MEASURED)) { | ||
246 | struct dentry *dentry = dget(path->dentry); | ||
247 | struct vfsmount *mnt = mntget(path->mnt); | ||
248 | |||
249 | file = dentry_open(dentry, mnt, O_RDONLY | O_LARGEFILE, | ||
250 | current_cred()); | ||
251 | if (IS_ERR(file)) { | ||
252 | int audit_info = 0; | ||
253 | |||
254 | integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, | ||
255 | dentry->d_name.name, | ||
256 | "add_measurement", | ||
257 | "dentry_open failed", | ||
258 | 1, audit_info); | ||
259 | file = NULL; | ||
260 | goto out; | ||
261 | } | ||
262 | rc = get_path_measurement(iint, file, dentry->d_name.name); | ||
263 | } | ||
264 | out: | ||
265 | mutex_unlock(&iint->mutex); | ||
266 | if (file) | ||
267 | fput(file); | ||
268 | kref_put(&iint->refcount, iint_free); | ||
269 | return 0; | ||
270 | } | ||
271 | EXPORT_SYMBOL_GPL(ima_path_check); | ||
272 | |||
273 | static int process_measurement(struct file *file, const unsigned char *filename, | 227 | static int process_measurement(struct file *file, const unsigned char *filename, |
274 | int mask, int function) | 228 | int mask, int function) |
275 | { | 229 | { |
@@ -297,33 +251,6 @@ out: | |||
297 | return rc; | 251 | return rc; |
298 | } | 252 | } |
299 | 253 | ||
300 | /* | ||
301 | * ima_counts_get - increment file counts | ||
302 | * | ||
303 | * - for IPC shm and shmat file. | ||
304 | * - for nfsd exported files. | ||
305 | * | ||
306 | * Increment the counts for these files to prevent unnecessary | ||
307 | * imbalance messages. | ||
308 | */ | ||
309 | void ima_counts_get(struct file *file) | ||
310 | { | ||
311 | struct inode *inode = file->f_dentry->d_inode; | ||
312 | struct ima_iint_cache *iint; | ||
313 | |||
314 | if (!ima_initialized || !S_ISREG(inode->i_mode)) | ||
315 | return; | ||
316 | iint = ima_iint_find_get(inode); | ||
317 | if (!iint) | ||
318 | return; | ||
319 | mutex_lock(&iint->mutex); | ||
320 | ima_inc_counts(iint, file->f_mode); | ||
321 | mutex_unlock(&iint->mutex); | ||
322 | |||
323 | kref_put(&iint->refcount, iint_free); | ||
324 | } | ||
325 | EXPORT_SYMBOL_GPL(ima_counts_get); | ||
326 | |||
327 | /** | 254 | /** |
328 | * ima_file_mmap - based on policy, collect/store measurement. | 255 | * ima_file_mmap - based on policy, collect/store measurement. |
329 | * @file: pointer to the file to be measured (May be NULL) | 256 | * @file: pointer to the file to be measured (May be NULL) |
@@ -369,6 +296,27 @@ int ima_bprm_check(struct linux_binprm *bprm) | |||
369 | return 0; | 296 | return 0; |
370 | } | 297 | } |
371 | 298 | ||
299 | /** | ||
300 | * ima_path_check - based on policy, collect/store measurement. | ||
301 | * @file: pointer to the file to be measured | ||
302 | * @mask: contains MAY_READ, MAY_WRITE or MAY_EXECUTE | ||
303 | * | ||
304 | * Measure files based on the ima_must_measure() policy decision. | ||
305 | * | ||
306 | * Always return 0 and audit dentry_open failures. | ||
307 | * (Return code will be based upon measurement appraisal.) | ||
308 | */ | ||
309 | int ima_path_check(struct file *file, int mask) | ||
310 | { | ||
311 | int rc; | ||
312 | |||
313 | rc = process_measurement(file, file->f_dentry->d_name.name, | ||
314 | mask & (MAY_READ | MAY_WRITE | MAY_EXEC), | ||
315 | PATH_CHECK); | ||
316 | return 0; | ||
317 | } | ||
318 | EXPORT_SYMBOL_GPL(ima_path_check); | ||
319 | |||
372 | static int __init init_ima(void) | 320 | static int __init init_ima(void) |
373 | { | 321 | { |
374 | int error; | 322 | int error; |