[cfe-dev] [llvm-dev] RFC: Implementing -fno-delete-null-pointer-checks in clang
James Y Knight via cfe-dev
cfe-dev at lists.llvm.org
Sun Apr 22 17:17:58 PDT 2018
On Sun, Apr 22, 2018, 5:11 AM David Chisnall via cfe-dev <
cfe-dev at lists.llvm.org> wrote:
> I disagree, because it depends on what you mean by dereference. If it is
> safe to dereference NULL, then that means that it is also safe to hoist the
> null dereference above the check. For example:
Nope. The intent is that NULL is *potentially* dereferenceable, just as any
other address would be. Not that it's known to *always* be dereferenceable.
if (x == NULL)
> y = *x;
> Is safe to transform into:
> y = *x;
> if (x == NULL)
> The load is guaranteed not to trap by the fact that NULL a dereferencable
That transform is still not safe with the new flag, because you do not know
that address 0 is dereferenceable in that code. You also don't know that
it's definitely _not_ dereferenceable, however -- which is the new behavior.
> Any code that is loading or storing from null prior to a null check
> probably doesn’t mind if the null check is then elided, because it’s going
> to trap anyway (supporting C code that allows loads and stores from an
> address with a bit pattern of 0 when interpreted as an integer is
> incredibly hard, as our friends at IBM can attest).
No -- the kernel *does* mind exactly this, because in some circumstances,
dereferencing null *does not trap* in kernel context. So you end up with
both no trap and no check, and a security vulnerability. That's less true
now, with the various other protections e.g. min_mmap_address sysctl, SMAP,
and others, but still, the desire is to keep this from happening.
Here is the commit which started using the flag in the kernel:
Author: Eugene Teo <eteo at redhat.com>
Date: Wed Jul 15 14:59:10 2009 +0800
Add '-fno-delete-null-pointer-checks' to gcc CFLAGS
Turning on this flag could prevent the compiler from optimising away
some "useless" checks for null pointers. Such bugs can sometimes become
exploitable at compile time because of the -O2 optimisation.
An example that clearly shows this 'problem' is commit 6bf67672.
static void __devexit agnx_pci_remove(struct pci_dev *pdev)
struct ieee80211_hw *dev = pci_get_drvdata(pdev);
- struct agnx_priv *priv = dev->priv;
+ struct agnx_priv *priv;
+ priv = dev->priv;
By reverting this patch, and compile it with and without
-fno-delete-null-pointer-checks flag, we can see that the check for dev
is compiled away.
call printk #
- testq %r12, %r12 # dev
- je .L94 #,
movq %r12, %rdi # dev,
Clearly the 'fix' is to stop using dev before it is tested, but building
with -fno-delete-null-pointer-checks flag at least makes it harder to
Teo <eugeneteo at kernel.sg>
Acked-by: Eric Paris <eparis at redhat.com>
Acked-by: Wang Cong <amwang at redhat.com>
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the cfe-dev