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

[Wang Zhaolong <wangzhaolong1@huawei.com>]

> …
>> +++ 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