Re: kernel crash in mknod

From: Paul Moore <paul@paul-moore.com>
To: Christian Brauner <brauner@kernel.org>
Cc: Roberto Sassu <roberto.sassu@huawei.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	 Steve French <smfrench@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	 linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	CIFS <linux-cifs@vger.kernel.org>,
	 Paulo Alcantara <pc@manguebit.com>,
	Christian Brauner <christian@brauner.io>,
	 Mimi Zohar <zohar@linux.ibm.com>,
	 "linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	 "linux-security-module@vger.kernel.org"
	<linux-security-module@vger.kernel.org>
Subject: Re: kernel crash in mknod
Date: Mon, 25 Mar 2024 13:21:20 -0400	[thread overview]
Message-ID: <CAHC9VhSamu4706AwP1M+3D9vyxrMZgaZmHH8KUmjuG1Jig4aPw@mail.gmail.com> (raw)
In-Reply-To: <20240325-beugen-kraftvoll-1390fd52d59c@brauner>

On Mon, Mar 25, 2024 at 12:06 PM Christian Brauner <brauner@kernel.org> wrote:
> I'm a bit confused now why this is taking a dentry. Nothing in IMA or
> EVM cares about the dentry for these hooks so it really should have take
> an inode in the first place?

I don't want to speak for Roberto or Mimi here, but this LSM hook was
intended to replace the dedicated ima_post_path_mknod() hook as I
wanted to see IMA/EVM integrated as proper LSMs so we could so away
with all of the special IMA/EVM hooks and treat everything as a LSM.
Part of this was creating new LSM hooks where historically we only had
a IMA and/or EVM hook, the security_path_post_mknod() hook is such a
case (e.g. /ima_post_path_mknod/security_path_post_mknod/) and the new
LSM hook kept the same parameters as the old IMA hook.

Yes, you are correct that neither IMA and EVM do anything with the
dentry other than looking at the associated inode.  I'm not the
IMA/EVM expert in this thread, but I suspect this is simply an old
vestige of former code, or perhaps an "optimization" to avoid having
to fetch the inode pointer in cases where IMA/EVM was not enabled
(although it is used in the vfs_create() call directly above, so who
knows ...

> And one minor other question I just realized. Why are some of the new
> hooks called security_path_post_mknod() when they aren't actually taking
> a path in contrast to say
> security_path_{chown,chmod,mknod,chroot,truncate}() that do.

Once again, think of this as a
/ima_post_path_mknod/security_path_post_mknod/ type of replacement and
you have your answer.  That said, I'm not really against bikeshedding
LSM hook names if people want to do that, it's not a stable protected
API so while we try to keep it stable~ish simply for our own sanity,
I'm happy to see it changed if everyone agrees it makes sense.

-- 
paul-moore.com