linux-kernel - Re: [PATCH] ovl: Check for NULL OVL

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: <202504211558.182D13B3@keescook>
Date: Mon, 21 Apr 2025 16:00:19 -0700
From: Kees Cook <kees@...nel.org>
To: Amir Goldstein <amir73il@...il.com>
Cc: Miklos Szeredi <miklos@...redi.hu>, linux-unionfs@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] ovl: Check for NULL OVL_E() results

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@...nel.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