Bug 199055 - KASAN: poison skb linear data tail
Summary: KASAN: poison skb linear data tail
Status: REOPENED
Alias: None
Product: Memory Management
Classification: Unclassified
Component: Sanitizers (show other bugs)
Hardware: All Linux
: P1 enhancement
Assignee: Dmitry Vyukov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-08 09:20 UTC by Dmitry Vyukov
Modified: 2025-04-30 03:26 UTC (History)
5 users (show)

See Also:
Kernel Version: ALL
Subsystem:
Regression: No
Bisected commit-id:


Attachments

Description Dmitry Vyukov 2018-03-08 09:20:21 UTC
Filing here so it's not get lost:

As far as I understand pskb_may_pull() plays important role in packet
parsing for all protocols. And we did custom fragmentation of packets
emitted via tun (IFF_NAPI_FRAGS). However, it seems that it does not
give any results (bugs found), and I think the reason for this is that
linear data is rounded up and is usually quite large. So if a parsing
function does pskb_may_pull(1), or does not do it at all, it can
usually access more and it will go unnoticed. KASAN has an ability to
do custom poisoning: it can poison/unpoison any memory range, and then
detect any reads/writes to that range. What do you think about adding
custom KASAN poisoning to pskb_may_pull() and switching it to
non-eager mode (pull only what was requested) under KASAN? Do you
think it has potential for finding important bugs? What amount of work
is this?
Comment 1 Stephen Hemminger 2018-03-08 15:47:50 UTC
Kernel bugzilla is only for upstream kernel code.
It is not used for design discussions (use mailing list) or custom code (not our problem).
Comment 2 Dmitry Vyukov 2018-03-08 15:57:40 UTC
This is all about upstream code. Bugzilla is better than mailing lists, because things get lost on kernel mailing lists too easily.
This is not in networking component. Please don't manage this bug.
Comment 3 Dmitry Vyukov 2025-04-28 07:43:09 UTC
The recently added CONFIG_FAIL_SKB_REALLOC does part of this. syzkaller should be able to use it with systematic fault injection facility (/proc/self/fail-nth).

Does CONFIG_FAIL_SKB_REALLOC cause exact size reallocation? Say, if tries to get 17 bytes, it should call kmalloc(17) so that KASAN can catch all out-of-bounds accesses at least when fault injection has happened.

With that extension it may give more-or-less everything we want.
Comment 4 Jakub Kicinski 2025-04-28 18:51:54 UTC
Hi! The initial target for FAIL_SKB_REALLOC was slightly different. The bug of interest was:

  struct hdr *hdr = skb->data;

  if (pskb_may_pull(skb, sizeof(*hdr)))
    drop;

  use(hdr->field);

the use() is UAF, because pskb_may_pull() can reallocate the underlying buffer.

You're saying basically make pskb_may_pull() do the opposite of what it normally does, and "truncate" the head strictly to only what was requested? Or just make it limit the skb buffer "rounding up" logic?
Comment 5 Dmitry Vyukov 2025-04-29 10:14:50 UTC
> You're saying basically make pskb_may_pull() do the opposite of what it
> normally does, and "truncate" the head strictly to only what was requested?
> Or just make it limit the skb buffer "rounding up" logic?

By "truncate" do you mean some logical limit within skb, or limit passed to kmalloc?
KASAN does not know about the internal limit within sbk, so it won't detect violations against it. KASAN has annotations to mark/unmark regions of memory as "poisoned" manually. But I am not sure how messy these annotations will be for skb (wrong annotations will lead to both false positives and false negatives).

So I was thinking about just asking for exact size from kmalloc on "fault injection".

I am not sure how expensive it will be to do this always when KASAN is enabled (both reallocation and exact size). KASAN already slows down execution by 2x, so it may be reasonable, and it should be a very useful mode for networking testing.

What do you think about a separate CONFIG that would enable this?
Comment 6 Stephen Hemminger 2025-04-29 14:48:23 UTC
Almost no kernel developers read bugzilla directly.
I am the gatekeeper and filter some bugs to netdev@vger.kernel.org
Comment 7 Dmitry Vyukov 2025-04-29 14:55:51 UTC
We actually use Bugzialla for this component, this is our up-to-date issue list:
https://bugzilla.kernel.org/buglist.cgi?bug_status=__open__&component=Sanitizers&product=Memory%20Management

Mailing lists don't have a shared notion of active/non-active status, so not particularly suitable for tracking.
Comment 8 Jakub Kicinski 2025-04-29 15:19:09 UTC
Dmitry, check out this diagram:
https://web.git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/tree/include/linux/skbuff.h#n734

We stuff some metadata after the tailroom so exact allocation won't help. Since we don't zalloc I presume KASAN will already know that tailroom is uninitialized?

As for config -- no preference. CONFIG_FAIL_SKB_REALLOC is meant for syzbot, too.
Comment 9 Dmitry Vyukov 2025-04-30 03:25:59 UTC
KASAN does not track initialized-ness of data.
KMSAN does, but it reports bugs only on "uses" of uninit values (not reads, results of some reads may be unusud later) + it does not catch out-of-bounds writes + KMSAN it significantly less used than KASAN.
Comment 10 Dmitry Vyukov 2025-04-30 03:26:12 UTC
KASAN does not track initialized-ness of data.
KMSAN does, but it reports bugs only on "uses" of uninit values (not reads, results of some reads may be unusud later) + it does not catch out-of-bounds writes + KMSAN it significantly less used than KASAN.

Note You need to log in before you can comment on or make changes to this bug.