Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1676153

Re: [PATCH v2] kref: Avoid null pointer dereference after WARN

From Daniel Micay <danielmicay@gmail.com>
Newsgroups linux.kernel
Subject Re: [PATCH v2] kref: Avoid null pointer dereference after WARN
Date 2017-06-27 22:20 +0200
Message-ID <tX57Q-5yg-29@gated-at.bofh.it> (permalink)
References <tX426-4PK-23@gated-at.bofh.it> <tX4ls-4ZQ-11@gated-at.bofh.it> <tX4lt-4ZQ-41@gated-at.bofh.it> <tX4v8-53A-3@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On Tue, 2017-06-27 at 12:34 -0700, Kees Cook wrote:
> On Tue, Jun 27, 2017 at 12:26 PM, Jason A. Donenfeld <Jason@zx2c4.com>
> wrote:
> > On Tue, Jun 27, 2017 at 9:22 PM, Andi Kleen <ak@linux.intel.com>
> > wrote:
> > > Who would actually set mman_min_addr incorrectly?
> > 
> > Historically there have been quite a few bypasses of mmap_min_addr,
> > actually. This is well-trodden ground.
> 
> Targeting things in /proc/sys via confused privileged helpers is
> extremely common. See Chrome OS pwn2own exploits (targetting modprobe
> sysctl), and plenty of others. Modern attack methodology is rarely a
> single-bug attack, but rather a chain of bugs, which may include
> producing or exploiting weak userspace configurations to soften the
> kernel.
> 
> Regardless, it's a fair point that checking this unconditionally is
> wasteful. Strangely this doesn't help:
> 
> -               BUG_ON(release == NULL);
> +               if (!__builtin_constant_p(release))
> +                       BUG_ON(release == NULL);
> 
> When nearly all callers pass a function directly:
> 
> ...
> drivers/block/rbd.c:            kref_put(&spec->kref, rbd_spec_free);
> drivers/char/hw_random/core.c:          kref_put(&rng->ref,
> cleanup_rng);
> drivers/char/ipmi/ipmi_msghandler.c:
> kref_put(&e->intf->refcount, intf_free);
> ...
> 
> Hmmm
> 
> -Kees

It doesn't mean the address is constant if there's a fixed function
being passed to it. It's not known at compile-time and if the code can
be relocated it's not known at link-time.

I don't personally care about checks like this but I split it out with
some others just because it was there already.

Clang has a nullability attribute which is similar to nonnull but it
doesn't cause UB when violated, so if GCC picked that up it could be
added all over the place as an annotation on parameters to trigger
warnings. There's a sanitizer for it, so it can be made to trap with
-fsanitize=nullability -fsanitize-trap=nullability.

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

[PATCH v2] kref: Avoid null pointer dereference after WARN Kees Cook <keescook@chromium.org> - 2017-06-27 21:10 +0200
  Re: [PATCH v2] kref: Avoid null pointer dereference after WARN "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-27 21:20 +0200
  Re: [PATCH v2] kref: Avoid null pointer dereference after WARN Andi Kleen <ak@linux.intel.com> - 2017-06-27 21:30 +0200
    Re: [PATCH v2] kref: Avoid null pointer dereference after WARN "Jason A. Donenfeld" <Jason@zx2c4.com> - 2017-06-27 21:30 +0200
      Re: [PATCH v2] kref: Avoid null pointer dereference after WARN Kees Cook <keescook@chromium.org> - 2017-06-27 21:40 +0200
        Re: [PATCH v2] kref: Avoid null pointer dereference after WARN Andi Kleen <ak@linux.intel.com> - 2017-06-27 21:50 +0200
        Re: [PATCH v2] kref: Avoid null pointer dereference after WARN Daniel Micay <danielmicay@gmail.com> - 2017-06-27 22:20 +0200

csiph-web