On Sun, Mar 24, 2024 at 04:50:24PM +0000, Roberto Sassu wrote:

quoted

From: Al Viro [mailto:viro@ftp.linux.org.uk] On Behalf Of Al Viro
Sent: Sunday, March 24, 2024 6:47 AM
On Sun, Mar 24, 2024 at 12:00:15AM -0500, Steve French wrote:

quoted

Anyone else seeing this kernel crash in do_mknodat (I see it with a
simple "mkfifo" on smb3 mount).  I started seeing this in 6.9-rc (did
not see it in 6.8).   I did not see it with the 3/12/23 mainline
(early in the 6.9-rc merge Window) but I do see it in the 3/22 build
so it looks like the regression was introduced by:

	FWIW, successful ->mknod() is allowed to return 0 and unhash
dentry, rather than bothering with lookups.  So commit in question
is bogus - lack of error does *NOT* mean that you have struct inode
existing, let alone attached to dentry.  That kind of behaviour
used to be common for network filesystems more than just for ->mknod(),
the theory being "if somebody wants to look at it, they can bloody
well pay the cost of lookup after dcache miss".

Said that, the language in D/f/vfs.rst is vague as hell and is very easy
to misread in direction of "you must instantiate".

Thankfully, there's no counterpart with mkdir - *there* it's not just
possible, it's inevitable in some cases for e.g. nfs.

What the hell is that hook doing in non-S_IFREG cases, anyway?  Move it
up and be done with it...

Hi Al

thanks for the patch. Indeed, it was like that before, when instead of
an LSM hook there was an IMA call.

Could you please start adding lore links into your commit messages for
all messages that are sent to a mailing list? It really makes tracking
down the original thread a lot easier.

However, I thought, since we were promoting it as an LSM hook,
we should be as generic possible, and support more usages than
what was needed for IMA.

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?

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.