[lkml]   [2017]   [May]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [kernel-hardening] Re: [PATCH v9 1/4] syscalls: Verify address limit before returning to user-mode
On Mon, May 08, 2017 at 09:33:52AM +0200, Ingo Molnar wrote:
> (added more Cc:s)
> * Thomas Garnier <> wrote:
> > On Fri, Apr 28, 2017 at 8:32 AM, Thomas Garnier <> wrote:
> > > Ensure that a syscall does not return to user-mode with a kernel address
> > > limit. If that happens, a process can corrupt kernel-mode memory and
> > > elevate privileges [1].
> > >
> > > The CONFIG_ADDR_LIMIT_CHECK option disables the generic check so each
> > > architecture can create optimized versions. This option is enabled by
> > > default on s390 because a similar feature already exists.
> > >
> > > [1]
> > >
> > > Signed-off-by: Thomas Garnier <>
> > > Tested-by: Kees Cook <>
> >
> > Ingo: Do you want to take the set?
> Yeah, so now I'm questioning the whole premise of the feature, sorry :-/
> A big disavantage is that the "security check" will add 2-5 instructions to the
> system call fast path. Every one of them, and essentially forever. Just to handle
> a CVE that was caused by a buggy touch-screen driver helper function leaking
> KERNEL_DS and which was fixed long ago ...
> And yes, I realize that there were other such bugs and that such bugs might occur
> in the future - but why not push the overhead of the security check to the kernel
> build phase? I.e. I'm wondering how well we could do static analysis during kernel
> build - would a limited mode of Sparse be good enough for that? Or we could add a
> new static checker to tools/, built from first principles and used primarily for
> extended syntactical checking.
> For example I'd consider it a good practice to mandate that if a kernel function
> sets KERNEL_DS then it must restore it as well. Any function that does not do
> that, or is too complex for the static analysis to prove correctness for sure
> should be considered buggy!
> Are there any common kernel APIs outside set_fs() that set KERNEL_DS
> intentionally? The overwhelming pattern ought to be:
> orig_fs = get_fs();
> set_fs(KERNEL_DS);
> ...
> set_fs(orig_fs);
> ... and even a relatively simple static analysis tool ought to be able to see
> through that.
> I'd even suggest we do it not like Sparse builds are done today, but in a more
> integrated fashion: do static analysis as part of a typical kernel defconfig build
> and not tolerate warnings but go for a 'zero warnings' policy like Linus uses for
> modconfig builds.
> _That_ solution I'd feel very, very good about - it would be so much better than
> any runtime checks...
> Not to mention that such an integrated static analysis facility would allow many
> other things to be checked during build time, which we couldn't possibly check
> runtime.

What about a simple coccinelle script to test for this type of thing?
We write it once, add it to the in-kernel body of tests, and then 0-day
runs it on all trees all the time. That should catch this type of
issue, like all of the other "bad programming bus" that the tool
currently catches.


greg k-h

 \ /
  Last update: 2017-05-08 14:47    [W:0.146 / U:1.144 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site