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

[Wang Zhaolong <wangzhaolong1@xxxxxxxxxx>]

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