| Message ID | 20240205182506.3569743-5-stefanb@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | evm: Support signatures on stacked filesystem | expand |
Hi Stefan, kernel test robot noticed the following build errors: [auto build test ERROR on zohar-integrity/next-integrity] [also build test ERROR on pcmoore-selinux/next linus/master v6.8-rc3 next-20240206] [cannot apply to mszeredi-vfs/overlayfs-next mszeredi-vfs/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Stefan-Berger/ima-Rename-backing_inode-to-real_inode/20240206-022848 base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity patch link: https://lore.kernel.org/r/20240205182506.3569743-5-stefanb%40linux.ibm.com patch subject: [PATCH v2 4/9] ima: Reset EVM status upon detecting changes to the real file config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20240206/202402062032.8kRzlrPA-lkp@intel.com/config) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240206/202402062032.8kRzlrPA-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402062032.8kRzlrPA-lkp@intel.com/ All errors (new ones prefixed by >>): security/integrity/ima/ima_main.c: In function 'process_measurement': >> security/integrity/ima/ima_main.c:303:58: error: 'D_REAL_METADATA' undeclared (first use in this function) 303 | D_REAL_METADATA))) | ^~~~~~~~~~~~~~~ security/integrity/ima/ima_main.c:303:58: note: each undeclared identifier is reported only once for each function it appears in vim +/D_REAL_METADATA +303 security/integrity/ima/ima_main.c 207 208 static int process_measurement(struct file *file, const struct cred *cred, 209 u32 secid, char *buf, loff_t size, int mask, 210 enum ima_hooks func) 211 { 212 struct inode *real_inode, *inode = file_inode(file); 213 struct integrity_iint_cache *iint = NULL; 214 struct ima_template_desc *template_desc = NULL; 215 char *pathbuf = NULL; 216 char filename[NAME_MAX]; 217 const char *pathname = NULL; 218 int rc = 0, action, must_appraise = 0; 219 int pcr = CONFIG_IMA_MEASURE_PCR_IDX; 220 struct evm_ima_xattr_data *xattr_value = NULL; 221 struct modsig *modsig = NULL; 222 int xattr_len = 0; 223 bool violation_check; 224 enum hash_algo hash_algo; 225 unsigned int allowed_algos = 0; 226 227 if (!ima_policy_flag || !S_ISREG(inode->i_mode)) 228 return 0; 229 230 /* Return an IMA_MEASURE, IMA_APPRAISE, IMA_AUDIT action 231 * bitmask based on the appraise/audit/measurement policy. 232 * Included is the appraise submask. 233 */ 234 action = ima_get_action(file_mnt_idmap(file), inode, cred, secid, 235 mask, func, &pcr, &template_desc, NULL, 236 &allowed_algos); 237 violation_check = ((func == FILE_CHECK || func == MMAP_CHECK || 238 func == MMAP_CHECK_REQPROT) && 239 (ima_policy_flag & IMA_MEASURE)); 240 if (!action && !violation_check) 241 return 0; 242 243 must_appraise = action & IMA_APPRAISE; 244 245 /* Is the appraise rule hook specific? */ 246 if (action & IMA_FILE_APPRAISE) 247 func = FILE_CHECK; 248 249 inode_lock(inode); 250 251 if (action) { 252 iint = integrity_inode_get(inode); 253 if (!iint) 254 rc = -ENOMEM; 255 } 256 257 if (!rc && violation_check) 258 ima_rdwr_violation_check(file, iint, action & IMA_MEASURE, 259 &pathbuf, &pathname, filename); 260 261 inode_unlock(inode); 262 263 if (rc) 264 goto out; 265 if (!action) 266 goto out; 267 268 mutex_lock(&iint->mutex); 269 270 if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags)) 271 /* reset appraisal flags if ima_inode_post_setattr was called */ 272 iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED | 273 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK | 274 IMA_NONACTION_FLAGS); 275 276 /* 277 * Re-evaulate the file if either the xattr has changed or the 278 * kernel has no way of detecting file change on the filesystem. 279 * (Limited to privileged mounted filesystems.) 280 */ 281 if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) || 282 ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) && 283 !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) && 284 !(action & IMA_FAIL_UNVERIFIABLE_SIGS))) { 285 iint->flags &= ~IMA_DONE_MASK; 286 iint->measured_pcrs = 0; 287 } 288 289 /* 290 * Detect and re-evaluate changes made to the inode holding file data. 291 */ 292 real_inode = d_real_inode(file_dentry(file)); 293 if (real_inode != inode && 294 (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) { 295 if (!IS_I_VERSION(real_inode) || 296 real_inode->i_sb->s_dev != iint->real_dev || 297 real_inode->i_ino != iint->real_ino || 298 !inode_eq_iversion(real_inode, iint->version)) { 299 iint->flags &= ~IMA_DONE_MASK; 300 iint->measured_pcrs = 0; 301 302 if (real_inode == d_inode(d_real(file_dentry(file), > 303 D_REAL_METADATA))) 304 evm_reset_cache_status(file_dentry(file), iint); 305 } 306 } 307 308 /* Determine if already appraised/measured based on bitmask 309 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED, 310 * IMA_AUDIT, IMA_AUDITED) 311 */ 312 iint->flags |= action; 313 action &= IMA_DO_MASK; 314 action &= ~((iint->flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1); 315 316 /* If target pcr is already measured, unset IMA_MEASURE action */ 317 if ((action & IMA_MEASURE) && (iint->measured_pcrs & (0x1 << pcr))) 318 action ^= IMA_MEASURE; 319 320 /* HASH sets the digital signature and update flags, nothing else */ 321 if ((action & IMA_HASH) && 322 !(test_bit(IMA_DIGSIG, &iint->atomic_flags))) { 323 xattr_len = ima_read_xattr(file_dentry(file), 324 &xattr_value, xattr_len); 325 if ((xattr_value && xattr_len > 2) && 326 (xattr_value->type == EVM_IMA_XATTR_DIGSIG)) 327 set_bit(IMA_DIGSIG, &iint->atomic_flags); 328 iint->flags |= IMA_HASHED; 329 action ^= IMA_HASH; 330 set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); 331 } 332 333 /* Nothing to do, just return existing appraised status */ 334 if (!action) { 335 if (must_appraise) { 336 rc = mmap_violation_check(func, file, &pathbuf, 337 &pathname, filename); 338 if (!rc) 339 rc = ima_get_cache_status(iint, func); 340 } 341 goto out_locked; 342 } 343 344 if ((action & IMA_APPRAISE_SUBMASK) || 345 strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) { 346 /* read 'security.ima' */ 347 xattr_len = ima_read_xattr(file_dentry(file), 348 &xattr_value, xattr_len); 349 350 /* 351 * Read the appended modsig if allowed by the policy, and allow 352 * an additional measurement list entry, if needed, based on the 353 * template format and whether the file was already measured. 354 */ 355 if (iint->flags & IMA_MODSIG_ALLOWED) { 356 rc = ima_read_modsig(func, buf, size, &modsig); 357 358 if (!rc && ima_template_has_modsig(template_desc) && 359 iint->flags & IMA_MEASURED) 360 action |= IMA_MEASURE; 361 } 362 } 363 364 hash_algo = ima_get_hash_algo(xattr_value, xattr_len); 365 366 rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig); 367 if (rc != 0 && rc != -EBADF && rc != -EINVAL) 368 goto out_locked; 369 370 if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */ 371 pathname = ima_d_path(&file->f_path, &pathbuf, filename); 372 373 if (action & IMA_MEASURE) 374 ima_store_measurement(iint, file, pathname, 375 xattr_value, xattr_len, modsig, pcr, 376 template_desc); 377 if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { 378 rc = ima_check_blacklist(iint, modsig, pcr); 379 if (rc != -EPERM) { 380 inode_lock(inode); 381 rc = ima_appraise_measurement(func, iint, file, 382 pathname, xattr_value, 383 xattr_len, modsig); 384 inode_unlock(inode); 385 } 386 if (!rc) 387 rc = mmap_violation_check(func, file, &pathbuf, 388 &pathname, filename); 389 } 390 if (action & IMA_AUDIT) 391 ima_audit_measurement(iint, pathname); 392 393 if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO)) 394 rc = 0; 395 396 /* Ensure the digest was generated using an allowed algorithm */ 397 if (rc == 0 && must_appraise && allowed_algos != 0 && 398 (allowed_algos & (1U << hash_algo)) == 0) { 399 rc = -EACCES; 400 401 integrity_audit_msg(AUDIT_INTEGRITY_DATA, file_inode(file), 402 pathname, "collect_data", 403 "denied-hash-algorithm", rc, 0); 404 } 405 out_locked: 406 if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) && 407 !(iint->flags & IMA_NEW_FILE)) 408 rc = -EACCES; 409 mutex_unlock(&iint->mutex); 410 kfree(xattr_value); 411 ima_free_modsig(modsig); 412 out: 413 if (pathbuf) 414 __putname(pathbuf); 415 if (must_appraise) { 416 if (rc && (ima_appraise & IMA_APPRAISE_ENFORCE)) 417 return -EACCES; 418 if (file->f_mode & FMODE_WRITE) 419 set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); 420 } 421 return 0; 422 } 423
On Mon, Feb 5, 2024 at 8:25 PM Stefan Berger <stefanb@linux.ibm.com> wrote: > > Piggyback the resetting of EVM status on IMA's file content detection that > is triggered when a not-yet-copied-up file on the 'lower' layer was > changed. However, since EVM only cares about changes to the file metadata, > only reset the EVM status if the 'lower' layer file is also the one holding > the file metadata. > > Note that in the case of a stacked filesystem (e.g., overlayfs) the iint > represents the file_inode() of a file on the overlay layer. The data in > the in iint must help detect file content (IMA) and file metadata (EVM) > changes occurring on the lower layer for as long as the content or > metadata have not been copied up yet. After copy-up the iit must continue > detecting them on the overlay layer. > > Changes to the file metadata on the overlay layer are causing an EVM > status reset through existing evm_inode_post_sattr/setxattr/removexattr > functions *if* an iint for a file exist. An iint exists if the file is > 'in (IMA) policy', meaning that IMA created an iint for the file's inode > since the file is covered by the IMA policy. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > --- > include/linux/evm.h | 8 ++++++++ > security/integrity/evm/evm_main.c | 7 +++++++ > security/integrity/ima/ima_main.c | 5 +++++ > 3 files changed, 20 insertions(+) > > diff --git a/include/linux/evm.h b/include/linux/evm.h > index 840ffbdc2860..eade9fff7d0b 100644 > --- a/include/linux/evm.h > +++ b/include/linux/evm.h > @@ -66,6 +66,8 @@ extern int evm_protected_xattr_if_enabled(const char *req_xattr_name); > extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, > int buffer_size, char type, > bool canonical_fmt); > +extern void evm_reset_cache_status(struct dentry *dentry, > + struct integrity_iint_cache *iint); > #ifdef CONFIG_FS_POSIX_ACL > extern int posix_xattr_acl(const char *xattrname); > #else > @@ -190,5 +192,11 @@ static inline int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, > return -EOPNOTSUPP; > } > > +static inline void evm_reset_cache_status(struct dentry *dentry, > + struct integrity_iint_cache *iint) > +{ > + return; > +} > + > #endif /* CONFIG_EVM */ > #endif /* LINUX_EVM_H */ > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index 565c36471408..81c94967f136 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -721,6 +721,13 @@ static void evm_reset_status(struct inode *inode) > iint->evm_status = INTEGRITY_UNKNOWN; > } > > +void evm_reset_cache_status(struct dentry *dentry, > + struct integrity_iint_cache *iint) > +{ > + if (d_real_inode(dentry) != d_backing_inode(dentry)) Is this really needed? You get here after checking (real_inode != inode) already > + iint->evm_status = INTEGRITY_UNKNOWN; > +} > + > /** > * evm_revalidate_status - report whether EVM status re-validation is necessary > * @xattr_name: pointer to the affected extended attribute name > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index f1a01d32b92a..b6ba829c4e67 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -26,6 +26,7 @@ > #include <linux/ima.h> > #include <linux/fs.h> > #include <linux/iversion.h> > +#include <linux/evm.h> > > #include "ima.h" > > @@ -297,6 +298,10 @@ static int process_measurement(struct file *file, const struct cred *cred, > !inode_eq_iversion(real_inode, iint->version)) { > iint->flags &= ~IMA_DONE_MASK; > iint->measured_pcrs = 0; > + > + if (real_inode == d_inode(d_real(file_dentry(file), > + D_REAL_METADATA))) > + evm_reset_cache_status(file_dentry(file), iint); Technically, you'd also need to store iint->real_meta_{dev,ino} when calculating EVM to be sure if the metadata inode had changed, because there is a possibility that file was not copied up yet, but the file is a metacopy in a middle layer and the lower data is in another layer. Think file metadata was copied from lower to upper layer, then the upper layer was made a middle layer and another upper layer added on top of it. In this situation, real_inode is in the lower layer, real_meta_inode is in the middle layer and after copy up of metadata, real_meta_inode will become in the upper layer. Not sure if this use case is interesting to EVM. Thanks, Amir.
Hi Stefan, kernel test robot noticed the following build errors: [auto build test ERROR on zohar-integrity/next-integrity] [also build test ERROR on pcmoore-selinux/next linus/master v6.8-rc3 next-20240206] [cannot apply to mszeredi-vfs/overlayfs-next mszeredi-vfs/next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Stefan-Berger/ima-Rename-backing_inode-to-real_inode/20240206-022848 base: https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity patch link: https://lore.kernel.org/r/20240205182506.3569743-5-stefanb%40linux.ibm.com patch subject: [PATCH v2 4/9] ima: Reset EVM status upon detecting changes to the real file config: x86_64-rhel-8.3-rust (https://download.01.org/0day-ci/archive/20240207/202402071226.lXIiGtbl-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240207/202402071226.lXIiGtbl-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202402071226.lXIiGtbl-lkp@intel.com/ All errors (new ones prefixed by >>): >> security/integrity/ima/ima_main.c:303:9: error: use of undeclared identifier 'D_REAL_METADATA' 303 | D_REAL_METADATA))) | ^ 1 error generated. vim +/D_REAL_METADATA +303 security/integrity/ima/ima_main.c 207 208 static int process_measurement(struct file *file, const struct cred *cred, 209 u32 secid, char *buf, loff_t size, int mask, 210 enum ima_hooks func) 211 { 212 struct inode *real_inode, *inode = file_inode(file); 213 struct integrity_iint_cache *iint = NULL; 214 struct ima_template_desc *template_desc = NULL; 215 char *pathbuf = NULL; 216 char filename[NAME_MAX]; 217 const char *pathname = NULL; 218 int rc = 0, action, must_appraise = 0; 219 int pcr = CONFIG_IMA_MEASURE_PCR_IDX; 220 struct evm_ima_xattr_data *xattr_value = NULL; 221 struct modsig *modsig = NULL; 222 int xattr_len = 0; 223 bool violation_check; 224 enum hash_algo hash_algo; 225 unsigned int allowed_algos = 0; 226 227 if (!ima_policy_flag || !S_ISREG(inode->i_mode)) 228 return 0; 229 230 /* Return an IMA_MEASURE, IMA_APPRAISE, IMA_AUDIT action 231 * bitmask based on the appraise/audit/measurement policy. 232 * Included is the appraise submask. 233 */ 234 action = ima_get_action(file_mnt_idmap(file), inode, cred, secid, 235 mask, func, &pcr, &template_desc, NULL, 236 &allowed_algos); 237 violation_check = ((func == FILE_CHECK || func == MMAP_CHECK || 238 func == MMAP_CHECK_REQPROT) && 239 (ima_policy_flag & IMA_MEASURE)); 240 if (!action && !violation_check) 241 return 0; 242 243 must_appraise = action & IMA_APPRAISE; 244 245 /* Is the appraise rule hook specific? */ 246 if (action & IMA_FILE_APPRAISE) 247 func = FILE_CHECK; 248 249 inode_lock(inode); 250 251 if (action) { 252 iint = integrity_inode_get(inode); 253 if (!iint) 254 rc = -ENOMEM; 255 } 256 257 if (!rc && violation_check) 258 ima_rdwr_violation_check(file, iint, action & IMA_MEASURE, 259 &pathbuf, &pathname, filename); 260 261 inode_unlock(inode); 262 263 if (rc) 264 goto out; 265 if (!action) 266 goto out; 267 268 mutex_lock(&iint->mutex); 269 270 if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags)) 271 /* reset appraisal flags if ima_inode_post_setattr was called */ 272 iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED | 273 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK | 274 IMA_NONACTION_FLAGS); 275 276 /* 277 * Re-evaulate the file if either the xattr has changed or the 278 * kernel has no way of detecting file change on the filesystem. 279 * (Limited to privileged mounted filesystems.) 280 */ 281 if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) || 282 ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) && 283 !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) && 284 !(action & IMA_FAIL_UNVERIFIABLE_SIGS))) { 285 iint->flags &= ~IMA_DONE_MASK; 286 iint->measured_pcrs = 0; 287 } 288 289 /* 290 * Detect and re-evaluate changes made to the inode holding file data. 291 */ 292 real_inode = d_real_inode(file_dentry(file)); 293 if (real_inode != inode && 294 (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) { 295 if (!IS_I_VERSION(real_inode) || 296 real_inode->i_sb->s_dev != iint->real_dev || 297 real_inode->i_ino != iint->real_ino || 298 !inode_eq_iversion(real_inode, iint->version)) { 299 iint->flags &= ~IMA_DONE_MASK; 300 iint->measured_pcrs = 0; 301 302 if (real_inode == d_inode(d_real(file_dentry(file), > 303 D_REAL_METADATA))) 304 evm_reset_cache_status(file_dentry(file), iint); 305 } 306 } 307 308 /* Determine if already appraised/measured based on bitmask 309 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED, 310 * IMA_AUDIT, IMA_AUDITED) 311 */ 312 iint->flags |= action; 313 action &= IMA_DO_MASK; 314 action &= ~((iint->flags & (IMA_DONE_MASK ^ IMA_MEASURED)) >> 1); 315 316 /* If target pcr is already measured, unset IMA_MEASURE action */ 317 if ((action & IMA_MEASURE) && (iint->measured_pcrs & (0x1 << pcr))) 318 action ^= IMA_MEASURE; 319 320 /* HASH sets the digital signature and update flags, nothing else */ 321 if ((action & IMA_HASH) && 322 !(test_bit(IMA_DIGSIG, &iint->atomic_flags))) { 323 xattr_len = ima_read_xattr(file_dentry(file), 324 &xattr_value, xattr_len); 325 if ((xattr_value && xattr_len > 2) && 326 (xattr_value->type == EVM_IMA_XATTR_DIGSIG)) 327 set_bit(IMA_DIGSIG, &iint->atomic_flags); 328 iint->flags |= IMA_HASHED; 329 action ^= IMA_HASH; 330 set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); 331 } 332 333 /* Nothing to do, just return existing appraised status */ 334 if (!action) { 335 if (must_appraise) { 336 rc = mmap_violation_check(func, file, &pathbuf, 337 &pathname, filename); 338 if (!rc) 339 rc = ima_get_cache_status(iint, func); 340 } 341 goto out_locked; 342 } 343 344 if ((action & IMA_APPRAISE_SUBMASK) || 345 strcmp(template_desc->name, IMA_TEMPLATE_IMA_NAME) != 0) { 346 /* read 'security.ima' */ 347 xattr_len = ima_read_xattr(file_dentry(file), 348 &xattr_value, xattr_len); 349 350 /* 351 * Read the appended modsig if allowed by the policy, and allow 352 * an additional measurement list entry, if needed, based on the 353 * template format and whether the file was already measured. 354 */ 355 if (iint->flags & IMA_MODSIG_ALLOWED) { 356 rc = ima_read_modsig(func, buf, size, &modsig); 357 358 if (!rc && ima_template_has_modsig(template_desc) && 359 iint->flags & IMA_MEASURED) 360 action |= IMA_MEASURE; 361 } 362 } 363 364 hash_algo = ima_get_hash_algo(xattr_value, xattr_len); 365 366 rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig); 367 if (rc != 0 && rc != -EBADF && rc != -EINVAL) 368 goto out_locked; 369 370 if (!pathbuf) /* ima_rdwr_violation possibly pre-fetched */ 371 pathname = ima_d_path(&file->f_path, &pathbuf, filename); 372 373 if (action & IMA_MEASURE) 374 ima_store_measurement(iint, file, pathname, 375 xattr_value, xattr_len, modsig, pcr, 376 template_desc); 377 if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) { 378 rc = ima_check_blacklist(iint, modsig, pcr); 379 if (rc != -EPERM) { 380 inode_lock(inode); 381 rc = ima_appraise_measurement(func, iint, file, 382 pathname, xattr_value, 383 xattr_len, modsig); 384 inode_unlock(inode); 385 } 386 if (!rc) 387 rc = mmap_violation_check(func, file, &pathbuf, 388 &pathname, filename); 389 } 390 if (action & IMA_AUDIT) 391 ima_audit_measurement(iint, pathname); 392 393 if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO)) 394 rc = 0; 395 396 /* Ensure the digest was generated using an allowed algorithm */ 397 if (rc == 0 && must_appraise && allowed_algos != 0 && 398 (allowed_algos & (1U << hash_algo)) == 0) { 399 rc = -EACCES; 400 401 integrity_audit_msg(AUDIT_INTEGRITY_DATA, file_inode(file), 402 pathname, "collect_data", 403 "denied-hash-algorithm", rc, 0); 404 } 405 out_locked: 406 if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) && 407 !(iint->flags & IMA_NEW_FILE)) 408 rc = -EACCES; 409 mutex_unlock(&iint->mutex); 410 kfree(xattr_value); 411 ima_free_modsig(modsig); 412 out: 413 if (pathbuf) 414 __putname(pathbuf); 415 if (must_appraise) { 416 if (rc && (ima_appraise & IMA_APPRAISE_ENFORCE)) 417 return -EACCES; 418 if (file->f_mode & FMODE_WRITE) 419 set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags); 420 } 421 return 0; 422 } 423
diff --git a/include/linux/evm.h b/include/linux/evm.h index 840ffbdc2860..eade9fff7d0b 100644 --- a/include/linux/evm.h +++ b/include/linux/evm.h @@ -66,6 +66,8 @@ extern int evm_protected_xattr_if_enabled(const char *req_xattr_name); extern int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, int buffer_size, char type, bool canonical_fmt); +extern void evm_reset_cache_status(struct dentry *dentry, + struct integrity_iint_cache *iint); #ifdef CONFIG_FS_POSIX_ACL extern int posix_xattr_acl(const char *xattrname); #else @@ -190,5 +192,11 @@ static inline int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer, return -EOPNOTSUPP; } +static inline void evm_reset_cache_status(struct dentry *dentry, + struct integrity_iint_cache *iint) +{ + return; +} + #endif /* CONFIG_EVM */ #endif /* LINUX_EVM_H */ diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 565c36471408..81c94967f136 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -721,6 +721,13 @@ static void evm_reset_status(struct inode *inode) iint->evm_status = INTEGRITY_UNKNOWN; } +void evm_reset_cache_status(struct dentry *dentry, + struct integrity_iint_cache *iint) +{ + if (d_real_inode(dentry) != d_backing_inode(dentry)) + iint->evm_status = INTEGRITY_UNKNOWN; +} + /** * evm_revalidate_status - report whether EVM status re-validation is necessary * @xattr_name: pointer to the affected extended attribute name diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index f1a01d32b92a..b6ba829c4e67 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -26,6 +26,7 @@ #include <linux/ima.h> #include <linux/fs.h> #include <linux/iversion.h> +#include <linux/evm.h> #include "ima.h" @@ -297,6 +298,10 @@ static int process_measurement(struct file *file, const struct cred *cred, !inode_eq_iversion(real_inode, iint->version)) { iint->flags &= ~IMA_DONE_MASK; iint->measured_pcrs = 0; + + if (real_inode == d_inode(d_real(file_dentry(file), + D_REAL_METADATA))) + evm_reset_cache_status(file_dentry(file), iint); } }
Piggyback the resetting of EVM status on IMA's file content detection that is triggered when a not-yet-copied-up file on the 'lower' layer was changed. However, since EVM only cares about changes to the file metadata, only reset the EVM status if the 'lower' layer file is also the one holding the file metadata. Note that in the case of a stacked filesystem (e.g., overlayfs) the iint represents the file_inode() of a file on the overlay layer. The data in the in iint must help detect file content (IMA) and file metadata (EVM) changes occurring on the lower layer for as long as the content or metadata have not been copied up yet. After copy-up the iit must continue detecting them on the overlay layer. Changes to the file metadata on the overlay layer are causing an EVM status reset through existing evm_inode_post_sattr/setxattr/removexattr functions *if* an iint for a file exist. An iint exists if the file is 'in (IMA) policy', meaning that IMA created an iint for the file's inode since the file is covered by the IMA policy. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- include/linux/evm.h | 8 ++++++++ security/integrity/evm/evm_main.c | 7 +++++++ security/integrity/ima/ima_main.c | 5 +++++ 3 files changed, 20 insertions(+)