[PATCH 0/3] kasan: memorize and print call_rcu stack

[Walter Wu]

This patchset improves KASAN reports by making them to have
call_rcu() call stack information. It is helpful for programmers
to solve use-after-free or double-free memory issue.

The KASAN report was as follows(cleaned up slightly):

BUG: KASAN: use-after-free in kasan_rcu_reclaim+0x58/0x60

Freed by task 0:
save_stack+0x24/0x50
__kasan_slab_free+0x110/0x178
kasan_slab_free+0x10/0x18
kfree+0x98/0x270
kasan_rcu_reclaim+0x1c/0x60
rcu_core+0x8b4/0x10f8
rcu_core_si+0xc/0x18
efi_header_end+0x238/0xa6c

First call_rcu() call stack:
save_stack+0x24/0x50
kasan_record_callrcu+0xc8/0xd8
call_rcu+0x190/0x580
kasan_rcu_uaf+0x1d8/0x278

Last call_rcu() call stack:
(stack is not available)


Add new CONFIG option to record first and last call_rcu() call stack
and KASAN report prints two call_rcu() call stack.

This option doesn't increase the cost of memory consumption. It is
only suitable for generic KASAN.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437

Walter Wu (3):
rcu/kasan: record and print call_rcu() call stack
kasan: record and print the free track
kasan: add KASAN_RCU_STACK_RECORD documentation

Documentation/dev-tools/kasan.rst | 21 +++++++++++++++++++++
include/linux/kasan.h | 7 +++++++
kernel/rcu/tree.c | 5 +++++
lib/Kconfig.kasan | 11 +++++++++++
mm/kasan/common.c | 31 +++++++++++++++++++++++++++++++
mm/kasan/kasan.h | 12 ++++++++++++
mm/kasan/report.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
7 files changed, 133 insertions(+), 7 deletions(-)

[Qian Cai]

I don’t understand why this needs to be a Kconfig option at all. If call_rcu() stacks are useful in general, then just always gather those information. How do developers judge if they need to select this option or not?

[Walter Wu]

Because we don't want to increase slub meta-data size, so enabling this
option can print call_rcu() stacks, but the in-use slub object doesn't
print free stack. So if have out-of-bound issue, then it will not print
free stack. It is a trade-off, see [1].

[1] https://bugzilla.kernel.org/show_bug.cgi?id=198437

Thanks

[Dmitry Vyukov]

Hi Walter,

Great you are tackling this!

I have the same general sentiment as Qian. I would enable this
unconditionally because:

1. We still can't get both rcu stack and free stack. I would assume
most kernel testing systems need to enable this (we definitely enable
on syzbot). This means we do not have free stack for allocation
objects in any reports coming from testing systems. Which greatly
diminishes the value of the other mode.

2. Kernel is undertested. Introducing any additional configuration
options is a problem in such context. Chances are that some of the
modes are not working or will break in future.

3. That free stack actually causes lots of confusion and I never found
it useful:
https://bugzilla.kernel.org/show_bug.cgi?id=198425
If it's a very delayed UAF, either one may get another report for the
same bug with not so delayed UAF, or if it's way too delayed, then the
previous free stack is wrong as well.

4. Most users don't care that much about debugging tools to learn
every bit of every debugging tool and spend time fine-tuning it for
their context. Most KASAN users won't even be aware of this choice,
and they will just use whatever is the default.

5. Each configuration option increases implementation complexity.

What would have value is if we figure out how to make both of them
work at the same time without increasing memory consumption. But I
don't see any way to do this.

I propose to make this the only mode. I am sure lots of users will
find this additional stack useful, whereas the free stack is even
frequently confusing.

[Walter Wu]

Ok.
If we want to have a default enabling it, but it should only work in
generic KASAN, because we need to get object status(allocation or
freeing) from shadow memory, tag-based KASAN can't do it. So we should
have a default enabling it in generic KASAN?

[Dmitry Vyukov]

Yes, let's do generic KASAN always memorizes rcu stack; tags KASAN
never memorizes rcu stacks. No new configurations.

[Walter Wu]

This patchset improves KASAN reports by making them to have

call_rcu() call stack information. It is useful for programmers

to solve use-after-free or double-free memory issue.

The KASAN report was as follows(cleaned up slightly):

BUG: KASAN: use-after-free in kasan_rcu_reclaim+0x58/0x60

Freed by task 0:
save_stack+0x24/0x50
__kasan_slab_free+0x110/0x178
kasan_slab_free+0x10/0x18
kfree+0x98/0x270
kasan_rcu_reclaim+0x1c/0x60
rcu_core+0x8b4/0x10f8
rcu_core_si+0xc/0x18
efi_header_end+0x238/0xa6c

First call_rcu() call stack:
save_stack+0x24/0x50
kasan_record_callrcu+0xc8/0xd8
call_rcu+0x190/0x580
kasan_rcu_uaf+0x1d8/0x278

Last call_rcu() call stack:
(stack is not available)

Generic KASAN will record first and last call_rcu() call stack
and print two call_rcu() call stack in KASAN report.

This feature doesn't increase the cost of memory consumption. It is

only suitable for generic KASAN.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Changes since v2:
- remove new config option, default enable it in generic KASAN
- test this feature in SLAB/SLUB, it is pass.
- modify macro to be more clearly
- modify documentation

Walter Wu (3):
rcu/kasan: record and print call_rcu() call stack
kasan: record and print the free track

kasan: update documentation for generic kasan

Documentation/dev-tools/kasan.rst | 6 ++++++
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 4 ++++
lib/Kconfig.kasan | 2 ++
mm/kasan/common.c | 26 ++++----------------------
mm/kasan/generic.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
mm/kasan/kasan.h | 23 +++++++++++++++++++++++
mm/kasan/report.c | 47 +++++++++++++++++++++--------------------------
mm/kasan/tags.c | 37 +++++++++++++++++++++++++++++++++++++
9 files changed, 149 insertions(+), 48 deletions(-)

[Dmitry Vyukov]

On Mon, May 11, 2020 at 4:24 AM Walter Wu <walter...@mediatek.com> wrote:
>
> This patchset improves KASAN reports by making them to have
> call_rcu() call stack information. It is useful for programmers
> to solve use-after-free or double-free memory issue.

Hi Walter,

I am looking at this now.

I've upload the change to gerrit [1]
https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458

I am not capable enough to meaningfully review such changes in this format...

[1] https://linux.googlesource.com/Documentation

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200511022359.15063-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

On Mon, 2020-05-11 at 12:01 +0200, 'Dmitry Vyukov' via kasan-dev wrote:
> On Mon, May 11, 2020 at 4:24 AM Walter Wu <walter...@mediatek.com> wrote:
> >
> > This patchset improves KASAN reports by making them to have
> > call_rcu() call stack information. It is useful for programmers
> > to solve use-after-free or double-free memory issue.
>
> Hi Walter,
>
> I am looking at this now.
>
> I've upload the change to gerrit [1]
> https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458
>
> I am not capable enough to meaningfully review such changes in this format...
>
> [1] https://linux.googlesource.com/Documentation
>

Hi Dmitry,

I don't fully understand your meaning, our patchset's format has
problem? or?

[Dmitry Vyukov]

On Mon, May 11, 2020 at 12:32 PM Walter Wu <walter...@mediatek.com> wrote:
>
> On Mon, 2020-05-11 at 12:01 +0200, 'Dmitry Vyukov' via kasan-dev wrote:
> > On Mon, May 11, 2020 at 4:24 AM Walter Wu <walter...@mediatek.com> wrote:
> > >
> > > This patchset improves KASAN reports by making them to have
> > > call_rcu() call stack information. It is useful for programmers
> > > to solve use-after-free or double-free memory issue.
> >
> > Hi Walter,
> >
> > I am looking at this now.
> >
> > I've upload the change to gerrit [1]
> > https://linux-review.googlesource.com/c/linux/kernel/git/torvalds/linux/+/2458
> >
> > I am not capable enough to meaningfully review such changes in this format...
> >
> > [1] https://linux.googlesource.com/Documentation
> >
>
> Hi Dmitry,
>
> I don't fully understand your meaning, our patchset's format has
> problem? or?

No, it does not have any problems. Your patch format is standard for kernel.

It's just complex patches in the standard kernel format that are hard
to review for me.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/1589193126.2930.2.camel%40mtksdccf07.

[Walter Wu]

This patchset improves KASAN reports by making them to have
call_rcu() call stack information. It is useful for programmers
to solve use-after-free or double-free memory issue.

The KASAN report was as follows(cleaned up slightly):

BUG: KASAN: use-after-free in kasan_rcu_reclaim+0x58/0x60

Freed by task 0:

kasan_save_stack+0x24/0x50
kasan_set_track+0x24/0x38
kasan_set_free_info+0x18/0x20
__kasan_slab_free+0x10c/0x170
kasan_slab_free+0x10/0x18
kfree+0x98/0x270
kasan_rcu_reclaim+0x1c/0x60

Last one call_rcu() call stack:
kasan_save_stack+0x24/0x50
kasan_record_aux_stack+0xbc/0xd0
call_rcu+0x8c/0x580
kasan_rcu_uaf+0xf4/0xf8

Generic KASAN will record the last two call_rcu() call stacks and
print up to 2 call_rcu() call stacks in KASAN report. it is only
suitable for generic KASAN.

This feature considers the size of struct kasan_alloc_meta and
kasan_free_meta, we try to optimize the structure layout and size
, let it get better memory consumption.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Changes since v2:
- remove new config option, default enable it in generic KASAN
- test this feature in SLAB/SLUB, it is pass.
- modify macro to be more clearly
- modify documentation

Changes since v3:
- change recording from first/last to the last two call stacks
- move free track into kasan free meta
- init slab_free_meta on object slot creation
- modify documentation

Walter Wu (4):

rcu/kasan: record and print call_rcu() call stack
kasan: record and print the free track

kasan: add tests for call_rcu stack recording

kasan: update documentation for generic kasan

Documentation/dev-tools/kasan.rst | 3 +++
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++
lib/Kconfig.kasan | 2 ++
lib/test_kasan.c | 30 ++++++++++++++++++++++++++++++
mm/kasan/common.c | 37 ++++++++++++-------------------------
mm/kasan/generic.c | 38 ++++++++++++++++++++++++++++++++++++++
mm/kasan/kasan.h | 17 +++++++++++++++++
mm/kasan/report.c | 36 ++++++++++++++++++++----------------
mm/kasan/tags.c | 37 +++++++++++++++++++++++++++++++++++++
10 files changed, 163 insertions(+), 41 deletions(-)

[Walter Wu]

Changes since v4:
- change variable name to be more clearly
- remove the redundant condition
- remove init free meta-data and increasing object condition

Walter Wu (4):
rcu/kasan: record and print call_rcu() call stack
kasan: record and print the free track
kasan: add tests for call_rcu stack recording
kasan: update documentation for generic kasan

Documentation/dev-tools/kasan.rst | 3 +++
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++
lib/Kconfig.kasan | 2 ++
lib/test_kasan.c | 30 ++++++++++++++++++++++++++++++

mm/kasan/common.c | 26 ++++----------------------
mm/kasan/generic.c | 37 +++++++++++++++++++++++++++++++++++++

mm/kasan/kasan.h | 17 +++++++++++++++++
mm/kasan/report.c | 36 ++++++++++++++++++++----------------
mm/kasan/tags.c | 37 +++++++++++++++++++++++++++++++++++++

10 files changed, 154 insertions(+), 38 deletions(-)

[Walter Wu]

Changes since v5:
- add a macro KASAN_KMALLOC_FREETRACK in order to check whether
print free stack
- change printing message
- remove descriptions in Kocong.kasan

Walter Wu (4):
rcu/kasan: record and print call_rcu() call stack
kasan: record and print the free track
kasan: add tests for call_rcu stack recording
kasan: update documentation for generic kasan

Documentation/dev-tools/kasan.rst | 3 +++
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++

lib/test_kasan.c | 30 ++++++++++++++++++++++++++++++
mm/kasan/common.c | 26 ++++----------------------

mm/kasan/generic.c | 43 +++++++++++++++++++++++++++++++++++++++++++
mm/kasan/generic_report.c | 1 +
mm/kasan/kasan.h | 23 +++++++++++++++++++++--
mm/kasan/quarantine.c | 1 +
mm/kasan/report.c | 50 ++++++++++++++++++++++++++++----------------------
mm/kasan/tags.c | 37 +++++++++++++++++++++++++++++++++++++
11 files changed, 172 insertions(+), 46 deletions(-)

[Walter Wu]

This patchset improves KASAN reports by making them to have
call_rcu() call stack information. It is useful for programmers
to solve use-after-free or double-free memory issue.

The KASAN report was as follows(cleaned up slightly):

BUG: KASAN: use-after-free in kasan_rcu_reclaim+0x58/0x60

Freed by task 0:
kasan_save_stack+0x24/0x50
kasan_set_track+0x24/0x38
kasan_set_free_info+0x18/0x20
__kasan_slab_free+0x10c/0x170
kasan_slab_free+0x10/0x18
kfree+0x98/0x270
kasan_rcu_reclaim+0x1c/0x60

Last call_rcu():

Changes since v6:
- reuse print_stack() in print_track()

Walter Wu (4):
rcu/kasan: record and print call_rcu() call stack
kasan: record and print the free track
kasan: add tests for call_rcu stack recording
kasan: update documentation for generic kasan

Documentation/dev-tools/kasan.rst | 3 +++
include/linux/kasan.h | 2 ++
kernel/rcu/tree.c | 2 ++
lib/test_kasan.c | 30 ++++++++++++++++++++++++++++++
mm/kasan/common.c | 26 ++++----------------------
mm/kasan/generic.c | 43 +++++++++++++++++++++++++++++++++++++++++++
mm/kasan/generic_report.c | 1 +
mm/kasan/kasan.h | 23 +++++++++++++++++++++--
mm/kasan/quarantine.c | 1 +

mm/kasan/report.c | 54 +++++++++++++++++++++++++++---------------------------
mm/kasan/tags.c | 37 +++++++++++++++++++++++++++++++++++++
11 files changed, 171 insertions(+), 51 deletions(-)

[Andrey Konovalov]

Reviewed-by: Andrey Konovalov <andre...@google.com>

for the series.

Thanks!

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20200522015757.22267-1-walter-zh.wu%40mediatek.com.

[Walter Wu]

This patchset improves KASAN reports by making them to have
call_rcu() call stack information. It is useful for programmers
to solve use-after-free or double-free memory issue.

The KASAN report was as follows(cleaned up slightly):

BUG: KASAN: use-after-free in kasan_rcu_reclaim+0x58/0x60

Freed by task 0:
kasan_save_stack+0x24/0x50
kasan_set_track+0x24/0x38
kasan_set_free_info+0x18/0x20
__kasan_slab_free+0x10c/0x170
kasan_slab_free+0x10/0x18
kfree+0x98/0x270
kasan_rcu_reclaim+0x1c/0x60

Last call_rcu():
kasan_save_stack+0x24/0x50
kasan_record_aux_stack+0xbc/0xd0
call_rcu+0x8c/0x580
kasan_rcu_uaf+0xf4/0xf8

Generic KASAN will record the last two call_rcu() call stacks and
print up to 2 call_rcu() call stacks in KASAN report. it is only
suitable for generic KASAN.

This feature considers the size of struct kasan_alloc_meta and
kasan_free_meta, we try to optimize the structure layout and size

, lets it get better memory consumption.

[1]https://bugzilla.kernel.org/show_bug.cgi?id=198437
[2]https://groups.google.com/forum/#!searchin/kasan-dev/better$20stack$20traces$20for$20rcu%7Csort:date/kasan-dev/KQsjT_88hDE/7rNUZprRBgAJ

Changes since v1:

- remove new config option, default enable it in generic KASAN
- test this feature in SLAB/SLUB, it is pass.
- modify macro to be more clearly
- modify documentation

Changes since v2:

- change recording from first/last to the last two call stacks
- move free track into kasan free meta
- init slab_free_meta on object slot creation
- modify documentation

Changes since v3:

- change variable name to be more clearly
- remove the redundant condition
- remove init free meta-data and increasing object condition

Changes since v4:

- add a macro KASAN_KMALLOC_FREETRACK in order to check whether
print free stack
- change printing message
- remove descriptions in Kocong.kasan

Changes since v5:

- reuse print_stack() in print_track()

Changes since v6:
- fix typo
- renamed the variable name in testcase

Walter Wu (4):
rcu: kasan: record and print call_rcu() call stack

[Walter Wu]

Hi Andrew,

Would you tell me why don't pick up this patches?
Do I miss something?

I will want to implement another new patches, but it need to depend on
this patches.


Thanks for your helps.

Walter

[Dmitry Vyukov]

On a related note.
Doing this for workqueue on top of these patches may be useful as
well, here is syzbot UAFs that mention process_one_work:
https://groups.google.com/forum/#!searchin/syzkaller-bugs/%22use-after-free%22$20process_one_work%7Csort:date

In some of these access/allocation happened in in process_one_work, in
some workqueue queueing stack may not add much.
But if we take the last one:
https://groups.google.com/forum/#!searchin/syzkaller-bugs/%22use-after-free%22$20process_one_work%7Csort:date/syzkaller-bugs/IYE0kt0BZMQ/zNM5rlzjAQAJ
It's exactly the same "free stack is useless" situation:

Freed by task 17:
kfree+0x10a/0x220 mm/slab.c:3757
process_one_work+0x76e/0xfd0 kernel/workqueue.c:2268
worker_thread+0xa7f/0x1450 kernel/workqueue.c:2414
kthread+0x353/0x380 kernel/kthread.c:268

The same may stand for times, I think I've seen some bugs where the
bad access happens in the timer as well.
Adding workqueue and timers should be pretty minimal change I think.

> > Documentation/dev-tools/kasan.rst | 3 +++
> > include/linux/kasan.h | 2 ++
> > kernel/rcu/tree.c | 2 ++
> > lib/test_kasan.c | 30 ++++++++++++++++++++++++++++++
> > mm/kasan/common.c | 26 ++++----------------------
> > mm/kasan/generic.c | 43 +++++++++++++++++++++++++++++++++++++++++++
> > mm/kasan/generic_report.c | 1 +
> > mm/kasan/kasan.h | 23 +++++++++++++++++++++--
> > mm/kasan/quarantine.c | 1 +
> > mm/kasan/report.c | 54 +++++++++++++++++++++++++++---------------------------
> > mm/kasan/tags.c | 37 +++++++++++++++++++++++++++++++++++++
> > 11 files changed, 171 insertions(+), 51 deletions(-)
>

> --
> You received this message because you are subscribed to the Google Groups "kasan-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/1592899732.13735.8.camel%40mtksdccf07.