2022-09-14 02:39:38

by Peter Collingbourne

[permalink] [raw]
Subject: [PATCH] kasan: call kasan_malloc() from __kmalloc_*track_caller()

We were failing to call kasan_malloc() from __kmalloc_*track_caller()
which was causing us to sometimes fail to produce KASAN error reports
for allocations made using e.g. devm_kcalloc(), as the KASAN poison was
not being initialized. Fix it.

Signed-off-by: Peter Collingbourne <[email protected]>
Cc: <[email protected]> # 5.15
---
The same problem is being fixed upstream in:
https://lore.kernel.org/all/[email protected]/
as part of a larger patch series, but this more targeted fix seems
more suitable for the stable kernel. Hyeonggon, maybe you can add
this patch to the start of your series and it can be picked up
by the stable maintainers.

mm/slub.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/mm/slub.c b/mm/slub.c
index 862dbd9af4f5..875c569c5cbe 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4926,6 +4926,8 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, unsigned long caller)
/* Honor the call site pointer we received. */
trace_kmalloc(caller, ret, s, size, s->size, gfpflags);

+ ret = kasan_kmalloc(s, ret, size, gfpflags);
+
return ret;
}
EXPORT_SYMBOL(__kmalloc_track_caller);
@@ -4957,6 +4959,8 @@ void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
/* Honor the call site pointer we received. */
trace_kmalloc_node(caller, ret, s, size, s->size, gfpflags, node);

+ ret = kasan_kmalloc(s, ret, size, gfpflags);
+
return ret;
}
EXPORT_SYMBOL(__kmalloc_node_track_caller);
--
2.37.2.789.g6183377224-goog


2022-09-14 07:43:10

by Hyeonggon Yoo

[permalink] [raw]
Subject: Re: [PATCH] kasan: call kasan_malloc() from __kmalloc_*track_caller()

On Tue, Sep 13, 2022 at 07:00:01PM -0700, Peter Collingbourne wrote:
> We were failing to call kasan_malloc() from __kmalloc_*track_caller()
> which was causing us to sometimes fail to produce KASAN error reports
> for allocations made using e.g. devm_kcalloc(), as the KASAN poison was
> not being initialized. Fix it.
>
> Signed-off-by: Peter Collingbourne <[email protected]>
> Cc: <[email protected]> # 5.15
> ---
> The same problem is being fixed upstream in:
> https://lore.kernel.org/all/[email protected]/
> as part of a larger patch series, but this more targeted fix seems
> more suitable for the stable kernel. Hyeonggon, maybe you can add
> this patch to the start of your series and it can be picked up
> by the stable maintainers.
>
> mm/slub.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 862dbd9af4f5..875c569c5cbe 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -4926,6 +4926,8 @@ void *__kmalloc_track_caller(size_t size, gfp_t gfpflags, unsigned long caller)
> /* Honor the call site pointer we received. */
> trace_kmalloc(caller, ret, s, size, s->size, gfpflags);
>
> + ret = kasan_kmalloc(s, ret, size, gfpflags);
> +
> return ret;
> }
> EXPORT_SYMBOL(__kmalloc_track_caller);
> @@ -4957,6 +4959,8 @@ void *__kmalloc_node_track_caller(size_t size, gfp_t gfpflags,
> /* Honor the call site pointer we received. */
> trace_kmalloc_node(caller, ret, s, size, s->size, gfpflags, node);
>
> + ret = kasan_kmalloc(s, ret, size, gfpflags);
> +
> return ret;
> }
> EXPORT_SYMBOL(__kmalloc_node_track_caller);
> --
> 2.37.2.789.g6183377224-goog
>

Ah, I should have sent it to stable team ;)

I think "Option 3" in Documentation/process/stable-kernel-rules.rst will be appropriate,
So will resend this after the series goes to Linus's tree.

Thank you Peter!

--
Thanks,
Hyeonggon

2022-09-16 18:13:48

by Vlastimil Babka

[permalink] [raw]
Subject: Re: [PATCH] kasan: call kasan_malloc() from __kmalloc_*track_caller()

On 9/14/22 08:52, Hyeonggon Yoo wrote:

Thanks for the Cc.

> On Tue, Sep 13, 2022 at 07:00:01PM -0700, Peter Collingbourne wrote:
>> We were failing to call kasan_malloc() from __kmalloc_*track_caller()
>> which was causing us to sometimes fail to produce KASAN error reports
>> for allocations made using e.g. devm_kcalloc(), as the KASAN poison was
>> not being initialized. Fix it.
>>
>> Signed-off-by: Peter Collingbourne <[email protected]>
>> Cc: <[email protected]> # 5.15
>> ---
>> The same problem is being fixed upstream in:

The "upstream" here is now only in -next, not mainline yet, so we still
have more options at this point.

>> https://lore.kernel.org/all/[email protected]/
>> as part of a larger patch series, but this more targeted fix seems
>> more suitable for the stable kernel. Hyeonggon, maybe you can add
>> this patch to the start of your series and it can be picked up
>> by the stable maintainers.
...
>
> Ah, I should have sent it to stable team ;)
>
> I think "Option 3" in Documentation/process/stable-kernel-rules.rst will be appropriate,
> So will resend this after the series goes to Linus's tree.

I'll pick this for sending to Linus after rc6, which means the series in
slab.git / -next will afterwards cause a trivial conflict to resolve
when merging. AFAIK Linus prefers that over late rebasing.
It will also make it simple for stable.

> Thank you Peter!
>

2022-09-19 12:34:05

by Hyeonggon Yoo

[permalink] [raw]
Subject: Re: [PATCH] kasan: call kasan_malloc() from __kmalloc_*track_caller()

On Fri, Sep 16, 2022 at 07:32:50PM +0200, Vlastimil Babka wrote:
> On 9/14/22 08:52, Hyeonggon Yoo wrote:
>
> Thanks for the Cc.
>
> > On Tue, Sep 13, 2022 at 07:00:01PM -0700, Peter Collingbourne wrote:
> >> We were failing to call kasan_malloc() from __kmalloc_*track_caller()
> >> which was causing us to sometimes fail to produce KASAN error reports
> >> for allocations made using e.g. devm_kcalloc(), as the KASAN poison was
> >> not being initialized. Fix it.
> >>
> >> Signed-off-by: Peter Collingbourne <[email protected]>
> >> Cc: <[email protected]> # 5.15
> >> ---
> >> The same problem is being fixed upstream in:
>
> The "upstream" here is now only in -next, not mainline yet, so we still
> have more options at this point.
>
> >> https://lore.kernel.org/all/[email protected]/
> >> as part of a larger patch series, but this more targeted fix seems
> >> more suitable for the stable kernel. Hyeonggon, maybe you can add
> >> this patch to the start of your series and it can be picked up
> >> by the stable maintainers.
> ...
> >
> > Ah, I should have sent it to stable team ;)
> >
> > I think "Option 3" in Documentation/process/stable-kernel-rules.rst will be appropriate,
> > So will resend this after the series goes to Linus's tree.
>
> I'll pick this for sending to Linus after rc6, which means the series in
> slab.git / -next will afterwards cause a trivial conflict to resolve
> when merging. AFAIK Linus prefers that over late rebasing.
> It will also make it simple for stable.

I think that is better option, thanks!

>
> > Thank you Peter!
> >
>

--
Thanks,
Hyeonggon