linux-kernel - Re: [PATCH] overlayfs: fix potential NULL pointer dereferences in file handle code

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] [day] [month] [year] [list]

Message-ID: <5e59d7ba-3b2a-40fb-b305-7b5a6316128f@huawei.com>
Date: Mon, 28 Apr 2025 21:01:16 +0800
From: Wang Zhaolong <wangzhaolong1@...wei.com>
To: Markus Elfring <Markus.Elfring@....de>, <linux-unionfs@...r.kernel.org>
CC: LKML <linux-kernel@...r.kernel.org>, Amir Goldstein <amir73il@...il.com>,
	Miklos Szeredi <miklos@...redi.hu>, Yang Erkun <yangerkun@...wei.com>, Zhang
 Yi <yi.zhang@...wei.com>
Subject: Re: [PATCH] overlayfs: fix potential NULL pointer dereferences in
 file handle code

> …
>> +++ b/fs/overlayfs/namei.c
>> @@ -496,10 +496,13 @@ static int ovl_verify_fh(struct ovl_fs *ofs, struct dentry *dentry,
>>   			 enum ovl_xattr ox, const struct ovl_fh *fh)
>>   {
>>   	struct ovl_fh *ofh = ovl_get_fh(ofs, dentry, ox);
>>   	int err = 0;
>>   
>> +	if (!fh)
>> +		return -ENODATA;
>> +
>>   	if (!ofh)
>>   		return -ENODATA;
> …
> 
> How do you think about to reduce the scope for these local variables
> (according to adjustment possibilities for input parameter validation)?
> 
> Regards,
> Markus

Hi Markus,

Thanks for your review!

Inspired by your suggestions, I would like to modify the approach as follows:

1. Postpone ofh initialization until after fh validation
2. Return -EINVAL for NULL fh (as invalid parameter rather than missing data)

```
@@ -493,13 +493,17 @@ static int ovl_check_origin(struct ovl_fs *ofs, struct dentry *upperdentry,
   * Return 0 on match, -ESTALE on mismatch, < 0 on error.
   */
  static int ovl_verify_fh(struct ovl_fs *ofs, struct dentry *dentry,
  			 enum ovl_xattr ox, const struct ovl_fh *fh)
  {
-	struct ovl_fh *ofh = ovl_get_fh(ofs, dentry, ox);
+	struct ovl_fh *ofh;
  	int err = 0;
  
+	if (!fh)
+		return -EINVAL;
+
+	ofh = ovl_get_fh(ofs, dentry, ox);
  	if (!ofh)
  		return -ENODATA;
  
  	if (IS_ERR(ofh))
  		return PTR_ERR(ofh);
```

3. Drop the unnecessary "&& fh" check in ovl_verify_set_fh() since NULL fh would
    return -EINVAL, not -ENODATA

This changes prevents unnecessary memory allocation and makes error handling more
precise.

What do you think of this modification? Does this approach work for you?

Regards,
Wang Zhaolong