On Tue, 2 Apr 2024 at 07:12, Roberto Sassu
<roberto.sassu@xxxxxxxxxxxxxxx> wrote:
> A single bug fix to address a kernel panic in the newly introduced function
> security_path_post_mknod.
So I've pulled from you before, but I still don't have a signature
chain for your key (not that I can even find the key itself, much less
a signature chain).
Last time I pulled, it was after having everybody else just verify the
actual commit.
This time, the commit looks like a valid "avoid NULL", but I have to
say that I also think the security layer code in question is ENTIRELY
WRONG.
IOW, as far as I can tell, the mknod() system call may indeed leave
the dentry unhashed, and rely on anybody who then wants to use the new
special file to just do a "lookup()" to actually use it.
HOWEVER.
That also means that the whole notion of "post_path_mknod() is
complete and utter hoghwash. There is not anything that the security
layer can possibly validly do.
End result: instead of checking the 'inode' for NULL, I think the
right fix is to remove that meaningless security hook. It cannot do
anything sane, since one option is always 'the inode hasn't been
initialized yet".
Put another way: any security hook that checks inode in
security_path_post_mknod() seems simply buggy.
But if we really want to do this ("if mknod creates a positive dentry,
I won't see it in lookup, so I want to appraise it now"), then we
should just deal with this in the generic layer with some hack like
this:
--- a/security/security.c
+++ b/security/security.c
@@ -1801,7 +1801,8 @@ 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))))
+ struct inode *inode = d_backing_inode(dentry);
+ if (unlikely(!inode || IS_PRIVATE(inode)))
return;
call_void_hook(path_post_mknod, idmap, dentry);
}
and IMA and EVM would have to do any validation at lookup() time for
the cases where the dentry wasn't hashed by ->mknod.
Anyway, all of this is to say that I don't feel like I can pull this without
(a) more acks by people
and
(b) explanations for why the simpler fix to just
security_path_post_mknod() isn't the right fix.
Linus
