lkml.org 
[lkml]   [2018]   [Aug]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 00/17] khwasan: kernel hardware assisted address sanitizer
    (trimming the quoted text a bit)

    On Thu, Aug 02, 2018 at 01:36:25PM +0200, Dmitry Vyukov wrote:
    > On Thu, Aug 2, 2018 at 1:10 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
    > > On Wed, Aug 01, 2018 at 06:52:09PM +0200, Dmitry Vyukov wrote:
    > >> On Wed, Aug 1, 2018 at 6:35 PM, Will Deacon <will.deacon@arm.com> wrote:
    > >> > I'd really like to enable pointer tagging in the kernel, I'm just still
    > >> > failing to see how we can do it in a controlled manner where we can reason
    > >> > about the semantic changes using something other than a best-effort,
    > >> > case-by-case basis which is likely to be fragile and error-prone.
    > >> > Unfortunately, if that's all we have, then this gets relegated to a
    > >> > debug feature, which sort of defeats the point in my opinion.
    > >>
    > >> Well, in some cases there is no other way as resorting to dynamic testing.
    > >> How do we ensure that kernel does not dereference NULL pointers, does
    > >> not access objects after free or out of bounds?
    > >
    > > We should not confuse software bugs (like NULL pointer dereference) with
    > > unexpected software behaviour introduced by khwasan where pointers no
    > > longer represent only an address range (e.g. calling find_vmap_area())
    > > but rather an address and a tag.
    [...]
    > > However, not untagging a pointer when converting to long may have
    > > side-effects in some cases and I consider these bugs introduced by the
    > > khwasan support rather than bugs in the original kernel code. Ideally
    > > we'd need some tooling on top of khwasan to detect such shortcomings but
    > > I'm not sure we can do this statically, as Andrey already mentioned. For
    > > __user pointers, things are slightly better as we can detect the
    > > conversion either with sparse (modified) or some LLVM changes.
    [...]
    > For example, LOCKDEP has the same problem. Previously correct code can
    > become incorrect and require finer-grained lock class annotations.
    > KMEMLEAK has the same problem: previously correct code that hides a
    > pointer may now need changes to unhide the pointer.

    It's not actually the same. Take the kmemleak example as I'm familiar
    with, previously correct code _continues_ to run correctly in the
    presence of kmemleak. The annotation or unhiding is only necessary to
    reduce the kmemleak false positives. With khwasan, OTOH, an explicit
    untagging is necessary so that the code functions correctly again.

    IOW, kmemleak only monitors the behaviour of the original code while
    khwasan changes such behaviour by tagging the pointers.

    > If somebody has a practical idea how to detect these statically, let's
    > do it. Otherwise let's go with the traditional solution to this --
    > dynamic testing. The patch series show that the problem is not a
    > disaster and we won't need to change just every line of kernel code.

    It's indeed not a disaster but we had to do this exercise to find out
    whether there are better ways of detecting where untagging is necessary.

    If you want to enable khwasan in "production" and since enabling it
    could potentially change the behaviour of existing code paths, the
    run-time validation space doubles as we'd need to get the same code
    coverage with and without the feature being enabled. I wouldn't say it's
    a blocker for khwasan, more like something to be aware of.

    The awareness is a bit of a problem as the normal programmer would have
    to pay more attention to conversions between pointer and long. Given
    that this is an arm64-only feature, we have a risk of khwasan-triggered
    bugs being introduced in generic code in the future (hence the
    suggestion of some static checker, if possible).

    --
    Catalin

    \
     
     \ /
      Last update: 2018-08-02 15:52    [W:4.646 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site