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 /security | |
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>
Diffstat (limited to 'security')
-rw-r--r-- | security/integrity/ima/ima_main.c | 236 |
1 files changed, 92 insertions, 144 deletions
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index a89f44d5e03..75aee18f616 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; |