From: Roberto Sassu roberto.sassu@huawei.com

Rename ima_post_path_mknod() and evm_post_path_mknod() respectively to ima_path_post_mknod() and evm_path_post_mknod(), to facilitate finding users of the path_post_mknod LSM hook.

Cc: stable@vger.kernel.org # 6.8.x Reported-by: Christian Brauner christian@brauner.io Closes: https://lore.kernel.org/linux-kernel/20240328-raushalten-krass-cb040068bde9@... Fixes: 05d1a717ec04 ("ima: add support for creating files using the mknodat syscall") Fixes: cd3cec0a02c7 ("ima: Move to LSM infrastructure") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com --- security/integrity/evm/evm_main.c | 4 ++-- security/integrity/ima/ima_main.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index ec1659273fcf..b4dd6e960203 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -1034,7 +1034,7 @@ static void evm_file_release(struct file *file) iint->flags &= ~EVM_NEW_FILE; }

-static void evm_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry) +static void evm_path_post_mknod(struct mnt_idmap *idmap, struct dentry *dentry) { struct inode *inode = d_backing_inode(dentry); struct evm_iint_cache *iint; @@ -1102,7 +1102,7 @@ static struct security_hook_list evm_hooks[] __ro_after_init = { LSM_HOOK_INIT(inode_init_security, evm_inode_init_security), LSM_HOOK_INIT(inode_alloc_security, evm_inode_alloc_security), LSM_HOOK_INIT(file_release, evm_file_release), - LSM_HOOK_INIT(path_post_mknod, evm_post_path_mknod), + LSM_HOOK_INIT(path_post_mknod, evm_path_post_mknod), };

static const struct lsm_id evm_lsmid = { diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index afc883e60cf3..f33124ceece3 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -709,14 +709,14 @@ static void ima_post_create_tmpfile(struct mnt_idmap *idmap, }

/** - * ima_post_path_mknod - mark as a new inode + * ima_path_post_mknod - mark as a new inode * @idmap: idmap of the mount the inode was found from * @dentry: newly created dentry * * Mark files created via the mknodat syscall as new, so that the * file data can be written later. */ -static void ima_post_path_mknod(struct mnt_idmap *idmap, struct dentry *dentry) +static void ima_path_post_mknod(struct mnt_idmap *idmap, struct dentry *dentry) { struct ima_iint_cache *iint; struct inode *inode = d_backing_inode(dentry); @@ -1165,7 +1165,7 @@ static struct security_hook_list ima_hooks[] __ro_after_init = { LSM_HOOK_INIT(kernel_post_load_data, ima_post_load_data), LSM_HOOK_INIT(kernel_read_file, ima_read_file), LSM_HOOK_INIT(kernel_post_read_file, ima_post_read_file), - LSM_HOOK_INIT(path_post_mknod, ima_post_path_mknod), + LSM_HOOK_INIT(path_post_mknod, ima_path_post_mknod), #ifdef CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS LSM_HOOK_INIT(key_post_create_or_update, ima_post_key_create_or_update), #endif

On Mar 29, 2024 Roberto Sassu roberto.sassu@huaweicloud.com wrote:

Commit 08abce60d63fi ("security: Introduce path_post_mknod hook") introduced security_path_post_mknod(), to replace the IMA-specific call to ima_post_path_mknod().

For symmetry with security_path_mknod(), security_path_post_mknod() is called after a successful mknod operation, for any file type, rather than only for regular files at the time there was the IMA call.

However, as reported by VFS maintainers, successful mknod operation does not mean that the dentry always has an inode attached to it (for example, not for FIFOs on a SAMBA mount).

If that condition happens, the kernel crashes when security_path_post_mknod() attempts to verify if the inode associated to the dentry is private.

Add an extra check to first verify if there is an inode attached to the dentry, before checking if the inode is private. Also add the same check to the current users of the path_post_mknod hook, ima_post_path_mknod() and evm_post_path_mknod().

Finally, use the proper helper, d_backing_inode(), to retrieve the inode from the dentry in ima_post_path_mknod().

Cc: stable@vger.kernel.org # 6.8.x Reported-by: Steve French smfrench@gmail.com Closes: https://lore.kernel.org/linux-kernel/CAH2r5msAVzxCUHHG8VKrMPUKQHmBpE6K9_vjhg... Fixes: 08abce60d63fi ("security: Introduce path_post_mknod hook") Signed-off-by: Roberto Sassu roberto.sassu@huawei.com Acked-by: Mimi Zohar zohar@linux.ibm.com

---

security/integrity/evm/evm_main.c | 6 ++++-- security/integrity/ima/ima_main.c | 5 +++-- security/security.c | 4 +++- 3 files changed, 10 insertions(+), 5 deletions(-)

In addition to the stable marking that Mimi already pointed out, I've got one small comment below, but otherwise this looks fine to me. Also, just to confirm, you're going to send patch 1/2 up to Linus during the v6.9-rc1 phase and hold patch 2/2 for the next merge window, right?

Acked-by: Paul Moore paul@paul-moore.com

diff --git a/security/security.c b/security/security.c index 7e118858b545..455f0749e1b0 100644 --- a/security/security.c +++ b/security/security.c @@ -1801,7 +1801,9 @@ EXPORT_SYMBOL(security_path_mknod); */ void security_path_post_mknod(struct mnt_idmap *idmap, struct dentry *dentry) {

• if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))

• /* Not all dentries have an inode attached after mknod. */
• if (d_backing_inode(dentry) &&

•    unlikely(IS_PRIVATE(d_backing_inode(dentry))))
  

  return;

I don't know how much impact this would have on the compiled code, but you could save yourself a call into d_backing_inode() by saving it to a local variable:

struct inode *inode = d_backing_inode(dentry); if (inode && unlikely(IS_PRIVATE(inode))) return;

call_void_hook(path_post_mknod, idmap, dentry); } -- 2.34.1

-- paul-moore.com