Re: [PATCH v6 00/11] Kernel address sanitizer - runtime memory debugger.

[Andrey Ryabinin]

2014-11-18 23:58 GMT+03:00 Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>:
> On Tue, 11 Nov 2014 10:21:42 +0300 Andrey Ryabinin <a.ryabinin@xxxxxxxxxxx> wrote:
>
>> Hi Andrew,
>>
>> Now we have stable GCC(4.9.2) which supports kasan and from my point of view patchset is ready for merging.
>> I could have sent v7 (it's just rebased v6), but I see no point in doing that and bothering people,
>> unless you are ready to take it.
>
> It's a huge pile of tricky code we'll need to maintain.  To justify its
> inclusion I think we need to be confident that kasan will find a
> significant number of significant bugs that
> kmemcheck/debug_pagealloc/slub_debug failed to detect.
>
> How do we get that confidence?  I've seen a small number of
> minorish-looking kasan-detected bug reports go past, maybe six or so.

I must admit that most bugs I've seen is a minor,
but there are  a bit more then six of them.

I've counted 16:

aab515d (fib_trie: remove potential out of bound access)
984f173 ([SCSI] sd: Fix potential out-of-bounds access)
5e9ae2e (aio: fix use-after-free in aio_migratepage)
2811eba (ipv6: udp packets following an UFO enqueued packet need also
be handled by UFO)
057db84 (tracing: Fix potential out-of-bounds in trace_get_user())
9709674 (ipv4: fix a race in ip4_datagram_release_cb())
4e8d213 (ext4: fix use-after-free in ext4_mb_new_blocks)
624483f (mm: rmap: fix use-after-free in __put_anon_vma)
93b7aca (lib/idr.c: fix out-of-bounds pointer dereference)
b4903d6 (mm: debugfs: move rounddown_pow_of_two() out from do_fault path)
40eea80 (net: sendmsg: fix NULL pointer dereference)
10ec947 (ipv4: fix buffer overflow in ip_options_compile())
dbf20cb2 (f2fs: avoid use invalid mapping of node_inode when evict meta inode)
d6d86c0 (mm/balloon_compaction: redesign ballooned pages management)

+ 2 recently found, seems minor:
    http://lkml.kernel.org/r/1415372020-1871-1-git-send-email-a.ryabinin@xxxxxxxxxxx
    (sched/numa: Fix out of bounds read in sched_init_numa())

    http://lkml.kernel.org/r/1415458085-12485-1-git-send-email-ryabinin.a.a@xxxxxxxxx
    (security: smack: fix out-of-bounds access in smk_parse_smack())

Note that some functionality is not yet implemented in this patch set.
Kasan has possibility
to detect out-of-bounds accesses on global/stack variables. Neither
kmemcheck/debug_pagealloc or slub_debug could do that.

> That's in a 20-year-old code base, so one new minor bug discovered per
> three years?  Not worth it!
>
> Presumably more bugs will be exposed as more people use kasan on
> different kernel configs, but will their number and seriousness justify
> the maintenance effort?
>

Yes, AFAIK there are only few users of kasan now, and I guess that
only small part of kernel code
was covered by it.
IMO kasan shouldn't take a lot maintenance efforts, most part of code
is isolated and it doesn't
have some complex dependencies on in-kernel API.
And you could always just poke me, I'd be happy to sort out any issues.

> If kasan will permit us to remove kmemcheck/debug_pagealloc/slub_debug
> then that tips the balance a little.  What's the feasibility of that?
>

I think kasan could replace kmemcheck at some point.
Unlike kmemcheck, kasan couldn't detect uninitialized memory reads now.
But It could be done  using the same compiler's instrumentation (I
have some proof-of-concept).
Though it will be a different Kconfig option, so you either enable
CONFIG_KASAN to detect out-of-bounds
and use-after-frees or CONFIG_DETECT_UNINITIALIZED_MEMORY to catch
only uninitialized memory reads.

Removing debug_pagealloc maybe is not so good idea, because it doesn't
eat much memory unlike kasan.

slub_debug could be enabled in production kernels without rebuilding,
so I wouldn't touch it too.

>
> Sorry to play the hardass here, but someone has to ;)
>


-- 
Best regards,
Andrey Ryabinin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/