linux-kernel - Re: kernel crash in mknod

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 28 Mar 2024 12:53:40 +0200
From: Roberto Sassu <roberto.sassu@...weicloud.com>
To: Christian Brauner <brauner@...nel.org>,
 Roberto Sassu <roberto.sassu@...wei.com>
Cc: Al Viro <viro@...iv.linux.org.uk>, Steve French <smfrench@...il.com>,
 LKML <linux-kernel@...r.kernel.org>,
 linux-fsdevel <linux-fsdevel@...r.kernel.org>,
 CIFS <linux-cifs@...r.kernel.org>, Paulo Alcantara <pc@...guebit.com>,
 Christian Brauner <christian@...uner.io>, Mimi Zohar <zohar@...ux.ibm.com>,
 Paul Moore <paul@...l-moore.com>,
 "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
 "linux-security-module@...r.kernel.org"
 <linux-security-module@...r.kernel.org>
Subject: Re: kernel crash in mknod

On 3/26/2024 12:40 PM, Christian Brauner wrote:
>> we can change the parameter of security_path_post_mknod() from
>> dentry to inode?
> 
> If all current callers only operate on the inode then it seems the best
> to only pass the inode. If there's some reason someone later needs a
> dentry the hook can always be changed.

Ok, so the crash is likely caused by:

void security_path_post_mknod(struct mnt_idmap *idmap, struct dentry 
*dentry)
{
         if (unlikely(IS_PRIVATE(d_backing_inode(dentry))))

I guess we can also simply check if there is an inode attached to the 
dentry, to minimize the changes. I can do both.

More technical question, do I need to do extra checks on the dentry 
before calling security_path_post_mknod()?

Thanks

Roberto

> For bigger changes it's also worthwhile if the object that's passed down
> into the hook-based LSM layer is as specific as possible. If someone
> does a change that affects lifetime rules of mounts then any hook that
> takes a struct path argument that's unused means going through each LSM
> that implements the hook only to find out it's not actually used.
> Similar for dentry vs inode imho.