Received-SPF: pass (google.com: domain of linux-kernel+bounces-122795-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <4a0b28ba-be57-4443-b91e-1a744a0feabf@huaweicloud.com>
Date: Thu, 28 Mar 2024 12:53:40 +0200
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: kernel crash in mknod
To: Christian Brauner <brauner@kernel.org>,
 Roberto Sassu <roberto.sassu@huawei.com>
Cc: 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>,
 Paul Moore <paul@paul-moore.com>,
 "linux-integrity@vger.kernel.org" <linux-integrity@vger.kernel.org>,
 "linux-security-module@vger.kernel.org"
 <linux-security-module@vger.kernel.org>
References: <CAH2r5msAVzxCUHHG8VKrMPUKQHmBpE6K9_vjhgDa1uAvwx4ppw@mail.gmail.com>
 <20240324054636.GT538574@ZenIV> <3441a4a1140944f5b418b70f557bca72@huawei.com>
 <20240325-beugen-kraftvoll-1390fd52d59c@brauner>
 <cb267d1c7988460094dbe19d1e7bcece@huawei.com>
 <20240326-halbkreis-wegstecken-8d5886e54d28@brauner>
Content-Language: en-US
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
In-Reply-To: <20240326-halbkreis-wegstecken-8d5886e54d28@brauner>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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.