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>

authored by Mimi Zohar and committed by Al Viro 8eb988c7 1e41568d

+97 -152
+2 -4
fs/namei.c
··· 1736 1736 if (nd.root.mnt) 1737 1737 path_put(&nd.root); 1738 1738 if (!IS_ERR(filp)) { 1739 - error = ima_path_check(&filp->f_path, filp->f_mode & 1740 - (MAY_READ | MAY_WRITE | MAY_EXEC)); 1739 + error = ima_path_check(filp, acc_mode); 1741 1740 if (error) { 1742 1741 fput(filp); 1743 1742 filp = ERR_PTR(error); ··· 1796 1797 } 1797 1798 filp = nameidata_to_filp(&nd); 1798 1799 if (!IS_ERR(filp)) { 1799 - error = ima_path_check(&filp->f_path, filp->f_mode & 1800 - (MAY_READ | MAY_WRITE | MAY_EXEC)); 1800 + error = ima_path_check(filp, acc_mode); 1801 1801 if (error) { 1802 1802 fput(filp); 1803 1803 filp = ERR_PTR(error);
+1 -2
fs/nfsd/vfs.c
··· 752 752 flags, current_cred()); 753 753 if (IS_ERR(*filp)) 754 754 host_err = PTR_ERR(*filp); 755 - host_err = ima_path_check(&(*filp)->f_path, 756 - access & (MAY_READ | MAY_WRITE | MAY_EXEC)); 755 + host_err = ima_path_check(*filp, access); 757 756 out_nfserr: 758 757 err = nfserrno(host_err); 759 758 out:
+2 -2
include/linux/ima.h
··· 17 17 extern int ima_bprm_check(struct linux_binprm *bprm); 18 18 extern int ima_inode_alloc(struct inode *inode); 19 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 21 extern void ima_file_free(struct file *file); 22 22 extern int ima_file_mmap(struct file *file, unsigned long prot); 23 23 extern void ima_counts_get(struct file *file); ··· 38 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 43 return 0; 44 44 }
+92 -144
security/integrity/ima/ima_main.c
··· 84 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 118 * Update the counts given an fmode_t 89 119 */ ··· 126 96 iint->readcount++; 127 97 if (mode & FMODE_WRITE) 128 98 iint->writecount++; 99 + } 100 + 101 + /* 102 + * ima_counts_get - increment file counts 103 + * 104 + * Maintain read/write counters for all files, but only 105 + * invalidate the PCR for measured files: 106 + * - Opening a file for write when already open for read, 107 + * results in a time of measure, time of use (ToMToU) error. 108 + * - Opening a file for read when already open for write, 109 + * could result in a file measurement error. 110 + * 111 + */ 112 + void ima_counts_get(struct file *file) 113 + { 114 + struct dentry *dentry = file->f_path.dentry; 115 + struct inode *inode = dentry->d_inode; 116 + fmode_t mode = file->f_mode; 117 + struct ima_iint_cache *iint; 118 + int rc; 119 + 120 + if (!ima_initialized || !S_ISREG(inode->i_mode)) 121 + return; 122 + iint = ima_iint_find_get(inode); 123 + if (!iint) 124 + return; 125 + mutex_lock(&iint->mutex); 126 + rc = ima_must_measure(iint, inode, MAY_READ, PATH_CHECK); 127 + if (rc < 0) 128 + goto out; 129 + 130 + if (mode & FMODE_WRITE) { 131 + ima_read_write_check(TOMTOU, iint, inode, dentry->d_name.name); 132 + goto out; 133 + } 134 + ima_read_write_check(OPEN_WRITERS, iint, inode, dentry->d_name.name); 135 + out: 136 + ima_inc_counts(iint, file->f_mode); 137 + mutex_unlock(&iint->mutex); 138 + 139 + kref_put(&iint->refcount, iint_free); 129 140 } 130 141 131 142 /* ··· 224 153 kref_put(&iint->refcount, iint_free); 225 154 } 226 155 227 - /* ima_read_write_check - reflect possible reading/writing errors in the PCR. 228 - * 229 - * When opening a file for read, if the file is already open for write, 230 - * the file could change, resulting in a file measurement error. 231 - * 232 - * Opening a file for write, if the file is already open for read, results 233 - * in a time of measure, time of use (ToMToU) error. 234 - * 235 - * In either case invalidate the PCR. 236 - */ 237 - enum iint_pcr_error { TOMTOU, OPEN_WRITERS }; 238 - static void ima_read_write_check(enum iint_pcr_error error, 239 - struct ima_iint_cache *iint, 240 - struct inode *inode, 241 - const unsigned char *filename) 242 - { 243 - switch (error) { 244 - case TOMTOU: 245 - if (iint->readcount > 0) 246 - ima_add_violation(inode, filename, "invalid_pcr", 247 - "ToMToU"); 248 - break; 249 - case OPEN_WRITERS: 250 - if (iint->writecount > 0) 251 - ima_add_violation(inode, filename, "invalid_pcr", 252 - "open_writers"); 253 - break; 254 - } 255 - } 256 - 257 - static int get_path_measurement(struct ima_iint_cache *iint, struct file *file, 258 - const unsigned char *filename) 259 - { 260 - int rc = 0; 261 - 262 - ima_inc_counts(iint, file->f_mode); 263 - 264 - rc = ima_collect_measurement(iint, file); 265 - if (!rc) 266 - ima_store_measurement(iint, file, filename); 267 - return rc; 268 - } 269 - 270 - /** 271 - * ima_path_check - based on policy, collect/store measurement. 272 - * @path: contains a pointer to the path to be measured 273 - * @mask: contains MAY_READ, MAY_WRITE or MAY_EXECUTE 274 - * 275 - * Measure the file being open for readonly, based on the 276 - * ima_must_measure() policy decision. 277 - * 278 - * Keep read/write counters for all files, but only 279 - * invalidate the PCR for measured files: 280 - * - Opening a file for write when already open for read, 281 - * results in a time of measure, time of use (ToMToU) error. 282 - * - Opening a file for read when already open for write, 283 - * could result in a file measurement error. 284 - * 285 - * Always return 0 and audit dentry_open failures. 286 - * (Return code will be based upon measurement appraisal.) 287 - */ 288 - int ima_path_check(struct path *path, int mask) 289 - { 290 - struct inode *inode = path->dentry->d_inode; 291 - struct ima_iint_cache *iint; 292 - struct file *file = NULL; 293 - int rc; 294 - 295 - if (!ima_initialized || !S_ISREG(inode->i_mode)) 296 - return 0; 297 - iint = ima_iint_find_get(inode); 298 - if (!iint) 299 - return 0; 300 - 301 - mutex_lock(&iint->mutex); 302 - 303 - rc = ima_must_measure(iint, inode, MAY_READ, PATH_CHECK); 304 - if (rc < 0) 305 - goto out; 306 - 307 - if ((mask & MAY_WRITE) || (mask == 0)) 308 - ima_read_write_check(TOMTOU, iint, inode, 309 - path->dentry->d_name.name); 310 - 311 - if ((mask & (MAY_WRITE | MAY_READ | MAY_EXEC)) != MAY_READ) 312 - goto out; 313 - 314 - ima_read_write_check(OPEN_WRITERS, iint, inode, 315 - path->dentry->d_name.name); 316 - if (!(iint->flags & IMA_MEASURED)) { 317 - struct dentry *dentry = dget(path->dentry); 318 - struct vfsmount *mnt = mntget(path->mnt); 319 - 320 - file = dentry_open(dentry, mnt, O_RDONLY | O_LARGEFILE, 321 - current_cred()); 322 - if (IS_ERR(file)) { 323 - int audit_info = 0; 324 - 325 - integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, 326 - dentry->d_name.name, 327 - "add_measurement", 328 - "dentry_open failed", 329 - 1, audit_info); 330 - file = NULL; 331 - goto out; 332 - } 333 - rc = get_path_measurement(iint, file, dentry->d_name.name); 334 - } 335 - out: 336 - mutex_unlock(&iint->mutex); 337 - if (file) 338 - fput(file); 339 - kref_put(&iint->refcount, iint_free); 340 - return 0; 341 - } 342 - EXPORT_SYMBOL_GPL(ima_path_check); 343 - 344 156 static int process_measurement(struct file *file, const unsigned char *filename, 345 157 int mask, int function) 346 158 { ··· 250 296 kref_put(&iint->refcount, iint_free); 251 297 return rc; 252 298 } 253 - 254 - /* 255 - * ima_counts_get - increment file counts 256 - * 257 - * - for IPC shm and shmat file. 258 - * - for nfsd exported files. 259 - * 260 - * Increment the counts for these files to prevent unnecessary 261 - * imbalance messages. 262 - */ 263 - void ima_counts_get(struct file *file) 264 - { 265 - struct inode *inode = file->f_dentry->d_inode; 266 - struct ima_iint_cache *iint; 267 - 268 - if (!ima_initialized || !S_ISREG(inode->i_mode)) 269 - return; 270 - iint = ima_iint_find_get(inode); 271 - if (!iint) 272 - return; 273 - mutex_lock(&iint->mutex); 274 - ima_inc_counts(iint, file->f_mode); 275 - mutex_unlock(&iint->mutex); 276 - 277 - kref_put(&iint->refcount, iint_free); 278 - } 279 - EXPORT_SYMBOL_GPL(ima_counts_get); 280 299 281 300 /** 282 301 * ima_file_mmap - based on policy, collect/store measurement. ··· 295 368 MAY_EXEC, BPRM_CHECK); 296 369 return 0; 297 370 } 371 + 372 + /** 373 + * ima_path_check - based on policy, collect/store measurement. 374 + * @file: pointer to the file to be measured 375 + * @mask: contains MAY_READ, MAY_WRITE or MAY_EXECUTE 376 + * 377 + * Measure files based on the ima_must_measure() policy decision. 378 + * 379 + * Always return 0 and audit dentry_open failures. 380 + * (Return code will be based upon measurement appraisal.) 381 + */ 382 + int ima_path_check(struct file *file, int mask) 383 + { 384 + int rc; 385 + 386 + rc = process_measurement(file, file->f_dentry->d_name.name, 387 + mask & (MAY_READ | MAY_WRITE | MAY_EXEC), 388 + PATH_CHECK); 389 + return 0; 390 + } 391 + EXPORT_SYMBOL_GPL(ima_path_check); 298 392 299 393 static int __init init_ima(void) 300 394 {