Re: [PATCH] ovl: Check for NULL OVL_E() results

[Kees Cook <kees@kernel.org>]

On Mon, Nov 18, 2024 at 07:20:52PM +0100, Amir Goldstein wrote:
> On Sun, Nov 17, 2024 at 5:46 AM Kees Cook <kees@kernel.org> wrote:
> >
> > GCC notices that it is possible for OVL_E() to return NULL (which
> > implies that d_inode(dentry) may be NULL).
> 
> I cannot follow this logic.
> 
> Yes, OVL_E() can be NULL, but
> it does not imply that inode is NULL, so if you think that
> code should to be fortified, what's wrong with:
> 
>  struct dentry *ovl_dentry_upper(struct dentry *dentry)
>  {
> -       return ovl_upperdentry_dereference(OVL_I(d_inode(dentry)));
> +       struct inode *inode = d_inode(dentry);
> +
> +       return inode ? ovl_upperdentry_dereference(OVL_I(inode)) : NULL;
>  }
> 
> TBH, I don't know where the line should be drawn for fortifying against
> future bugs, but if the goal of this patch is to silene a compiler warning
> then please specify this in the commit message, because I don't think
> there is any evidence of an actual bug, is there?

Sorry for the delay on this! I'm finally coming back around to these
fixes. :)

Yes, your suggestion works very nicely! That entirely solves the GCC
warning.

And correct, this was to deal with an over-eager compiler warning --
there was no bug here that I'm aware of.

I will send an updated patch with your suggestion.

-Kees

-- 
Kees Cook