On Sun, Mar 24, 2024 at 12:50 PM Roberto Sassu [off-list ref] 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.

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.

quoted

diff --git a/fs/namei.c b/fs/namei.c
index ceb9ddf8dfdd..821fe0e3f171 100644
--- a/fs/namei.c
+++ b/fs/namei.c

@@ -4050,6 +4050,8 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
              case 0: case S_IFREG:
                      error = vfs_create(idmap, path.dentry->d_inode,
                                         dentry, mode, true);
+                     if (!error)
+                             error = security_path_post_mknod(idmap, dentry);

Minor issue, security_path_post_mknod() does not return an error.

Also, please update the description of security_path_post_mknod() to say
that it is not going to be called for non-regular files.

Hopefully, Paul also agrees with this change.

Other than that, please add my:

Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>

No objections here for obvious reasons.

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

-- 
paul-moore.com