diff mbox series

[2/2] ima: evm: Rename *_post_path_mknod() to *_path_post_mknod()

Message ID 20240329105609.1566309-2-roberto.sassu@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series [1/2] security: Handle dentries without inode in security_path_post_mknod() | expand

Commit Message

Roberto Sassu March 29, 2024, 10:56 a.m. UTC 
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@brauner/
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(-)

Comments

Mimi Zohar March 29, 2024, 3:16 p.m. UTC | #1 
On Fri, 2024-03-29 at 11:56 +0100, Roberto Sassu wrote:
> 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

Since commit cd3cec0a02c7 ("ima: Move to LSM infrastructure") was upstreamed in
this open window.  This change does not need to be packported and should be
limited to IMA and EVM full fledge LSMs.

> Reported-by: Christian Brauner <christian@brauner.io>
> Closes: 
> https://lore.kernel.org/linux-kernel/20240328-raushalten-krass-cb040068bde9@brauner/
> Fixes: 05d1a717ec04 ("ima: add support for creating files using the mknodat
> syscall")

"Fixes: 05d1a717ec04" should be removed.

> Fixes: cd3cec0a02c7 ("ima: Move to LSM infrastructure")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Acked-by: Mimi Zohar <zohar@linux.ibm.com>
Roberto Sassu March 29, 2024, 3:45 p.m. UTC | #2 
On 3/29/2024 4:16 PM, Mimi Zohar wrote:
> On Fri, 2024-03-29 at 11:56 +0100, Roberto Sassu wrote:
>> 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
> 
> Since commit cd3cec0a02c7 ("ima: Move to LSM infrastructure") was upstreamed in
> this open window.  This change does not need to be packported and should be
> limited to IMA and EVM full fledge LSMs.

Yes, got it wrong.

>> Reported-by: Christian Brauner <christian@brauner.io>
>> Closes:
>> https://lore.kernel.org/linux-kernel/20240328-raushalten-krass-cb040068bde9@brauner/
>> Fixes: 05d1a717ec04 ("ima: add support for creating files using the mknodat
>> syscall")
> 
> "Fixes: 05d1a717ec04" should be removed.

Ok, I agree that it is not a necessary fix for stable kernels. We can 
reconsider it if there is a bug fix depending on it.

Thanks

Roberto

>> Fixes: cd3cec0a02c7 ("ima: Move to LSM infrastructure")
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> 
> Acked-by: Mimi Zohar <zohar@linux.ibm.com>
>
Paul Moore March 29, 2024, 7:12 p.m. UTC | #3 
On Fri, Mar 29, 2024 at 11:17 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2024-03-29 at 11:56 +0100, Roberto Sassu wrote:
> > 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
>
> Since commit cd3cec0a02c7 ("ima: Move to LSM infrastructure") was upstreamed in
> this open window.  This change does not need to be packported and should be
> limited to IMA and EVM full fledge LSMs.
>
> > Reported-by: Christian Brauner <christian@brauner.io>
> > Closes:
> > https://lore.kernel.org/linux-kernel/20240328-raushalten-krass-cb040068bde9@brauner/
> > Fixes: 05d1a717ec04 ("ima: add support for creating files using the mknodat
> > syscall")
>
> "Fixes: 05d1a717ec04" should be removed.

I'd take it one step further and remove both 'Fixes' tags.  A 'Fixes'
tag implies a flaw in the functionality of the code, this is just a
function rename.

Another important thing to keep in mind about 'Fixes' tags, unless
you've told the stable kernel folks to only take patches that you've
explicitly marked for stable, they are likely going to attempt to
backport anything with a 'Fixes' tag.

Regardless, since I was looking at 1/2 I took a quick look at this
patch and it looks fine to me once the comments have been
incorporated.

Reviewed-by: Paul Moore <paul@paul-moore.com>

> > Fixes: cd3cec0a02c7 ("ima: Move to LSM infrastructure")
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>
> Acked-by: Mimi Zohar <zohar@linux.ibm.com>
Mimi Zohar March 29, 2024, 7:27 p.m. UTC | #4 
[Cc: Sasha, Greg]

On Fri, 2024-03-29 at 15:12 -0400, Paul Moore wrote:
> I'd take it one step further and remove both 'Fixes' tags.  A 'Fixes'
> tag implies a flaw in the functionality of the code, this is just a
> function rename.

Totally agree.

> Another important thing to keep in mind about 'Fixes' tags, unless
> you've told the stable kernel folks to only take patches that you've
> explicitly marked for stable, they are likely going to attempt to
> backport anything with a 'Fixes' tag.

How do we go about doing that?  Do we just send an email to stable?

Is it disabled for security?  I thought new functionality won't be backported. 
Hopefully the changes for making IMA & EVM full fledged LSMs won't be
automatically backported to stable.

thanks,

Mimi
Paul Moore March 29, 2024, 7:56 p.m. UTC | #5 
On Fri, Mar 29, 2024 at 3:28 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2024-03-29 at 15:12 -0400, Paul Moore wrote:
> > Another important thing to keep in mind about 'Fixes' tags, unless
> > you've told the stable kernel folks to only take patches that you've
> > explicitly marked for stable, they are likely going to attempt to
> > backport anything with a 'Fixes' tag.
>
> How do we go about doing that?  Do we just send an email to stable?

When I asked for a change to the stable policy, it was an email
exchange with Greg where we setup what is essentially a shell glob to
filter out the files to skip unless explicitly tagged:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/stable-queue.git/tree/ignore_list

> Is it disabled for security?

I asked for it to be disabled for the LSM layer, SELinux, and audit.
I sent a note about it last year to the mailing list:

https://lore.kernel.org/linux-security-module/CAHC9VhQgzshziG2tvaQMd9jchAVMu39M4Ym9RCComgbXj+WF0Q@mail.gmail.com

> I thought new functionality won't be backported.

One thing I noticed fairly consistently in the trees I maintained is
that commits marked with a 'Fixes' tag were generally backported
regardless of if they were marked for stable.

> Hopefully the changes for making IMA & EVM full fledged LSMs won't be
> automatically backported to stable.

I haven't seen that happening, and I wouldn't expect it in the future
as none of those patches were explicitly marked for stable or had a
'Fixes' tag.

--
paul-moore.com
diff mbox series

Patch

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