* KASAN reported an error while executing accept-reust.t testcase
@ 2025-01-11 14:07 lizetao
2025-01-11 17:13 ` Jens Axboe
0 siblings, 1 reply; 10+ messages in thread
From: lizetao @ 2025-01-11 14:07 UTC (permalink / raw)
To: io-uring; +Cc: Jens Axboe, Pavel Begunkov
Hi all,
When I run the testcase liburing/accept-reust.t with CONFIG_KASAN=y and CONFIG_KASAN_EXTRA_INFO=y, I got
a error reported by KASAN:
Unable to handle kernel paging request at virtual address 00000c6455008008
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00000001104c5000
[00000c6455008008] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 6 UID: 0 PID: 352 Comm: kworker/u128:5 Not tainted 6.13.0-rc6-g0a2cb793507d #5
Hardware name: linux,dummy-virt (DT)
Workqueue: iou_exit io_ring_exit_work
pstate: 10000005 (nzcV daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : __kasan_mempool_unpoison_object+0x38/0x170
lr : io_netmsg_cache_free+0x8c/0x180
sp : ffff800083297a90
x29: ffff800083297a90 x28: ffffd4d7f67e88e4 x27: 0000000000000003
x26: 1fffe5958011502e x25: ffff2cabff976c18 x24: 1fffe5957ff2ed83
x23: ffff2cabff976c10 x22: 00000c6455008000 x21: 0002992540200001
x20: 0000000000000000 x19: 00000c6455008000 x18: 00000000489683f8
x17: ffffd4d7f68006ac x16: ffffd4d7f67eb3e0 x15: ffffd4d7f67e88e4
x14: ffffd4d7f766deac x13: ffffd4d7f6619030 x12: ffff7a9b012e3e26
x11: 1ffffa9b012e3e25 x10: ffff7a9b012e3e25 x9 : ffffd4d7f766debc
x8 : ffffd4d80971f128 x7 : 0000000000000001 x6 : 00008564fed1c1db
x5 : ffffd4d80971f128 x4 : ffff7a9b012e3e26 x3 : ffff2cabff976c00
x2 : ffffc1ffc0000000 x1 : 0000000000000000 x0 : 0002992540200001
Call trace:
__kasan_mempool_unpoison_object+0x38/0x170 (P)
io_netmsg_cache_free+0x8c/0x180
io_ring_exit_work+0xd4c/0x13a0
process_one_work+0x52c/0x1000
worker_thread+0x830/0xdc0
kthread+0x2bc/0x348
ret_from_fork+0x10/0x20
Code: aa0003f5 aa0103f4 8b131853 aa1303f6 (f9400662)
---[ end trace 0000000000000000 ]---
I preliminary analyzed the accept and connect code logic. In the accept-reuse.t testcase, kmsg->free_iov is
not used, so when calling io_netmsg_cache_free(), the kasan_mempool_unpoison_object(kmsg->free_iov...) path
should not be executed.
I used the hardware watchpoint to capture the first scene of modifying kmsg->free_iov:
Thread 3 hit Hardware watchpoint 7: *0xffff0000ebfc5410
Old value = 0
New value = -211812350
kasan_set_track (stack=<optimized out>, track=<optimized out>) at ./arch/arm64/include/asm/current.h:21
21 return (struct task_struct *)sp_el0;
# bt
kasan_set_track
kasan_save_track
kasan_save_free_info
poison_slab_object
__kasan_mempool_poison_object
kasan_mempool_poison_object
io_alloc_cache_put
io_netmsg_recycle
io_req_msg_cleanup
io_connect
io_issue_sqe
io_queue_sqe
io_req_task_submit
...
It's a bit strange. It was modified by KASAN. I can't understand this.
Maybe I missed something? Please let me know. Thanks.
---
Li Zetao
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: KASAN reported an error while executing accept-reust.t testcase
2025-01-11 14:07 KASAN reported an error while executing accept-reust.t testcase lizetao
@ 2025-01-11 17:13 ` Jens Axboe
2025-01-12 6:45 ` lizetao
0 siblings, 1 reply; 10+ messages in thread
From: Jens Axboe @ 2025-01-11 17:13 UTC (permalink / raw)
To: lizetao, io-uring; +Cc: Pavel Begunkov
On 1/11/25 7:07 AM, lizetao wrote:
> Hi all,
>
> When I run the testcase liburing/accept-reust.t with CONFIG_KASAN=y and CONFIG_KASAN_EXTRA_INFO=y, I got
> a error reported by KASAN:
Looks more like you get KASAN crashing...
> Unable to handle kernel paging request at virtual address 00000c6455008008
> Mem abort info:
> ESR = 0x0000000096000004
> EC = 0x25: DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> FSC = 0x04: level 0 translation fault
> Data abort info:
> ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> user pgtable: 4k pages, 48-bit VAs, pgdp=00000001104c5000
> [00000c6455008008] pgd=0000000000000000, p4d=0000000000000000
> Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 6 UID: 0 PID: 352 Comm: kworker/u128:5 Not tainted 6.13.0-rc6-g0a2cb793507d #5
> Hardware name: linux,dummy-virt (DT)
> Workqueue: iou_exit io_ring_exit_work
> pstate: 10000005 (nzcV daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : __kasan_mempool_unpoison_object+0x38/0x170
> lr : io_netmsg_cache_free+0x8c/0x180
> sp : ffff800083297a90
> x29: ffff800083297a90 x28: ffffd4d7f67e88e4 x27: 0000000000000003
> x26: 1fffe5958011502e x25: ffff2cabff976c18 x24: 1fffe5957ff2ed83
> x23: ffff2cabff976c10 x22: 00000c6455008000 x21: 0002992540200001
> x20: 0000000000000000 x19: 00000c6455008000 x18: 00000000489683f8
> x17: ffffd4d7f68006ac x16: ffffd4d7f67eb3e0 x15: ffffd4d7f67e88e4
> x14: ffffd4d7f766deac x13: ffffd4d7f6619030 x12: ffff7a9b012e3e26
> x11: 1ffffa9b012e3e25 x10: ffff7a9b012e3e25 x9 : ffffd4d7f766debc
> x8 : ffffd4d80971f128 x7 : 0000000000000001 x6 : 00008564fed1c1db
> x5 : ffffd4d80971f128 x4 : ffff7a9b012e3e26 x3 : ffff2cabff976c00
> x2 : ffffc1ffc0000000 x1 : 0000000000000000 x0 : 0002992540200001
> Call trace:
> __kasan_mempool_unpoison_object+0x38/0x170 (P)
> io_netmsg_cache_free+0x8c/0x180
> io_ring_exit_work+0xd4c/0x13a0
> process_one_work+0x52c/0x1000
> worker_thread+0x830/0xdc0
> kthread+0x2bc/0x348
> ret_from_fork+0x10/0x20
> Code: aa0003f5 aa0103f4 8b131853 aa1303f6 (f9400662)
> ---[ end trace 0000000000000000 ]---
>
>
> I preliminary analyzed the accept and connect code logic. In the
> accept-reuse.t testcase, kmsg->free_iov is not used, so when calling
> io_netmsg_cache_free(), the
> kasan_mempool_unpoison_object(kmsg->free_iov...) path should not be
> executed.
>
>
> I used the hardware watchpoint to capture the first scene of modifying kmsg->free_iov:
>
> Thread 3 hit Hardware watchpoint 7: *0xffff0000ebfc5410
> Old value = 0
> New value = -211812350
> kasan_set_track (stack=<optimized out>, track=<optimized out>) at ./arch/arm64/include/asm/current.h:21
> 21 return (struct task_struct *)sp_el0;
>
> # bt
> kasan_set_track
> kasan_save_track
> kasan_save_free_info
> poison_slab_object
> __kasan_mempool_poison_object
> kasan_mempool_poison_object
> io_alloc_cache_put
> io_netmsg_recycle
> io_req_msg_cleanup
> io_connect
> io_issue_sqe
> io_queue_sqe
> io_req_task_submit
> ...
>
>
> It's a bit strange. It was modified by KASAN. I can't understand this.
> Maybe I missed something? Please let me know. Thanks.
Looks like KASAN with the extra info ends up writing to
io_async_msghdr->free_iov somehow. No idea... For the test case in
question, ->free_iov should be NULL when initially allocated, and the
io_uring code isn't storing to it. Yet it's non-NULL when you later go
and free it, after calling kasan_mempool_poison_object().
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: KASAN reported an error while executing accept-reust.t testcase
2025-01-11 17:13 ` Jens Axboe
@ 2025-01-12 6:45 ` lizetao
2025-01-22 13:49 ` Andrey Ryabinin
0 siblings, 1 reply; 10+ messages in thread
From: lizetao @ 2025-01-12 6:45 UTC (permalink / raw)
To: Jens Axboe, io-uring
Cc: Pavel Begunkov, [email protected], [email protected],
[email protected]
Hi,
> -----Original Message-----
> From: Jens Axboe <[email protected]>
> Sent: Sunday, January 12, 2025 1:13 AM
> To: lizetao <[email protected]>; io-uring <[email protected]>
> Cc: Pavel Begunkov <[email protected]>
> Subject: Re: KASAN reported an error while executing accept-reust.t testcase
>
> On 1/11/25 7:07 AM, lizetao wrote:
> > Hi all,
> >
> > When I run the testcase liburing/accept-reust.t with CONFIG_KASAN=y
> > and CONFIG_KASAN_EXTRA_INFO=y, I got a error reported by KASAN:
>
> Looks more like you get KASAN crashing...
>
> > Unable to handle kernel paging request at virtual address
> > 00000c6455008008 Mem abort info:
> > ESR = 0x0000000096000004
> > EC = 0x25: DABT (current EL), IL = 32 bits
> > SET = 0, FnV = 0
> > EA = 0, S1PTW = 0
> > FSC = 0x04: level 0 translation fault Data abort info:
> > ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> > CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 user pgtable: 4k pages,
> > 48-bit VAs, pgdp=00000001104c5000 [00000c6455008008]
> > pgd=0000000000000000, p4d=0000000000000000 Internal error: Oops:
> > 0000000096000004 [#1] PREEMPT SMP Modules linked in:
> > CPU: 6 UID: 0 PID: 352 Comm: kworker/u128:5 Not tainted
> > 6.13.0-rc6-g0a2cb793507d #5 Hardware name: linux,dummy-virt (DT)
> > Workqueue: iou_exit io_ring_exit_work
> > pstate: 10000005 (nzcV daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc :
> > __kasan_mempool_unpoison_object+0x38/0x170
> > lr : io_netmsg_cache_free+0x8c/0x180
> > sp : ffff800083297a90
> > x29: ffff800083297a90 x28: ffffd4d7f67e88e4 x27: 0000000000000003
> > x26: 1fffe5958011502e x25: ffff2cabff976c18 x24: 1fffe5957ff2ed83
> > x23: ffff2cabff976c10 x22: 00000c6455008000 x21: 0002992540200001
> > x20: 0000000000000000 x19: 00000c6455008000 x18: 00000000489683f8
> > x17: ffffd4d7f68006ac x16: ffffd4d7f67eb3e0 x15: ffffd4d7f67e88e4
> > x14: ffffd4d7f766deac x13: ffffd4d7f6619030 x12: ffff7a9b012e3e26
> > x11: 1ffffa9b012e3e25 x10: ffff7a9b012e3e25 x9 : ffffd4d7f766debc
> > x8 : ffffd4d80971f128 x7 : 0000000000000001 x6 : 00008564fed1c1db
> > x5 : ffffd4d80971f128 x4 : ffff7a9b012e3e26 x3 : ffff2cabff976c00
> > x2 : ffffc1ffc0000000 x1 : 0000000000000000 x0 : 0002992540200001 Call
> > trace:
> > __kasan_mempool_unpoison_object+0x38/0x170 (P)
> > io_netmsg_cache_free+0x8c/0x180
> > io_ring_exit_work+0xd4c/0x13a0
> > process_one_work+0x52c/0x1000
> > worker_thread+0x830/0xdc0
> > kthread+0x2bc/0x348
> > ret_from_fork+0x10/0x20
> > Code: aa0003f5 aa0103f4 8b131853 aa1303f6 (f9400662) ---[ end trace
> > 0000000000000000 ]---
> >
> >
> > I preliminary analyzed the accept and connect code logic. In the
> > accept-reuse.t testcase, kmsg->free_iov is not used, so when calling
> > io_netmsg_cache_free(), the
> > kasan_mempool_unpoison_object(kmsg->free_iov...) path should not be
> > executed.
> >
> >
> > I used the hardware watchpoint to capture the first scene of modifying kmsg-
> >free_iov:
> >
> > Thread 3 hit Hardware watchpoint 7: *0xffff0000ebfc5410 Old value = 0
> > New value = -211812350 kasan_set_track (stack=<optimized out>,
> > track=<optimized out>) at ./arch/arm64/include/asm/current.h:21
> > 21 return (struct task_struct *)sp_el0;
> >
> > # bt
> > kasan_set_track
> > kasan_save_track
> > kasan_save_free_info
> > poison_slab_object
> > __kasan_mempool_poison_object
> > kasan_mempool_poison_object
> > io_alloc_cache_put
> > io_netmsg_recycle
> > io_req_msg_cleanup
> > io_connect
> > io_issue_sqe
> > io_queue_sqe
> > io_req_task_submit
> > ...
> >
> >
> > It's a bit strange. It was modified by KASAN. I can't understand this.
> > Maybe I missed something? Please let me know. Thanks.
>
> Looks like KASAN with the extra info ends up writing to io_async_msghdr-
> >free_iov somehow. No idea... For the test case in question, ->free_iov should
> be NULL when initially allocated, and the io_uring code isn't storing to it. Yet
> it's non-NULL when you later go and free it, after calling
> kasan_mempool_poison_object().
I also think so and would Juntong and Ryabinin or others KASAN developers be interested
In this problem?
+CC [email protected], [email protected] and [email protected]
Thank you so mush.
>
> --
> Jens Axboe
---
Li Zetao
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: KASAN reported an error while executing accept-reust.t testcase
2025-01-12 6:45 ` lizetao
@ 2025-01-22 13:49 ` Andrey Ryabinin
2025-01-22 16:06 ` [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects Andrey Ryabinin
0 siblings, 1 reply; 10+ messages in thread
From: Andrey Ryabinin @ 2025-01-22 13:49 UTC (permalink / raw)
To: lizetao
Cc: Jens Axboe, io-uring, Pavel Begunkov, [email protected],
[email protected], Andrey Konovalov
On Sun, Jan 12, 2025 at 7:45 AM lizetao <[email protected]> wrote:
>
> Hi,
>
> > -----Original Message-----
> > From: Jens Axboe <[email protected]>
> > Sent: Sunday, January 12, 2025 1:13 AM
> > To: lizetao <[email protected]>; io-uring <[email protected]>
> > Cc: Pavel Begunkov <[email protected]>
> > Subject: Re: KASAN reported an error while executing accept-reust.t testcase
> >
> > On 1/11/25 7:07 AM, lizetao wrote:
> > > Hi all,
> > >
> > > When I run the testcase liburing/accept-reust.t with CONFIG_KASAN=y
> > > and CONFIG_KASAN_EXTRA_INFO=y, I got a error reported by KASAN:
> >
> > Looks more like you get KASAN crashing...
> >
> > > Unable to handle kernel paging request at virtual address
> > > 00000c6455008008 Mem abort info:
> > > ESR = 0x0000000096000004
> > > EC = 0x25: DABT (current EL), IL = 32 bits
> > > SET = 0, FnV = 0
> > > EA = 0, S1PTW = 0
> > > FSC = 0x04: level 0 translation fault Data abort info:
> > > ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> > > CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> > > GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 user pgtable: 4k pages,
> > > 48-bit VAs, pgdp=00000001104c5000 [00000c6455008008]
> > > pgd=0000000000000000, p4d=0000000000000000 Internal error: Oops:
> > > 0000000096000004 [#1] PREEMPT SMP Modules linked in:
> > > CPU: 6 UID: 0 PID: 352 Comm: kworker/u128:5 Not tainted
> > > 6.13.0-rc6-g0a2cb793507d #5 Hardware name: linux,dummy-virt (DT)
> > > Workqueue: iou_exit io_ring_exit_work
> > > pstate: 10000005 (nzcV daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc :
> > > __kasan_mempool_unpoison_object+0x38/0x170
> > > lr : io_netmsg_cache_free+0x8c/0x180
> > > sp : ffff800083297a90
> > > x29: ffff800083297a90 x28: ffffd4d7f67e88e4 x27: 0000000000000003
> > > x26: 1fffe5958011502e x25: ffff2cabff976c18 x24: 1fffe5957ff2ed83
> > > x23: ffff2cabff976c10 x22: 00000c6455008000 x21: 0002992540200001
> > > x20: 0000000000000000 x19: 00000c6455008000 x18: 00000000489683f8
> > > x17: ffffd4d7f68006ac x16: ffffd4d7f67eb3e0 x15: ffffd4d7f67e88e4
> > > x14: ffffd4d7f766deac x13: ffffd4d7f6619030 x12: ffff7a9b012e3e26
> > > x11: 1ffffa9b012e3e25 x10: ffff7a9b012e3e25 x9 : ffffd4d7f766debc
> > > x8 : ffffd4d80971f128 x7 : 0000000000000001 x6 : 00008564fed1c1db
> > > x5 : ffffd4d80971f128 x4 : ffff7a9b012e3e26 x3 : ffff2cabff976c00
> > > x2 : ffffc1ffc0000000 x1 : 0000000000000000 x0 : 0002992540200001 Call
> > > trace:
> > > __kasan_mempool_unpoison_object+0x38/0x170 (P)
> > > io_netmsg_cache_free+0x8c/0x180
> > > io_ring_exit_work+0xd4c/0x13a0
> > > process_one_work+0x52c/0x1000
> > > worker_thread+0x830/0xdc0
> > > kthread+0x2bc/0x348
> > > ret_from_fork+0x10/0x20
> > > Code: aa0003f5 aa0103f4 8b131853 aa1303f6 (f9400662) ---[ end trace
> > > 0000000000000000 ]---
> > >
> > >
> > > I preliminary analyzed the accept and connect code logic. In the
> > > accept-reuse.t testcase, kmsg->free_iov is not used, so when calling
> > > io_netmsg_cache_free(), the
> > > kasan_mempool_unpoison_object(kmsg->free_iov...) path should not be
> > > executed.
> > >
> > >
> > > I used the hardware watchpoint to capture the first scene of modifying kmsg-
> > >free_iov:
> > >
> > > Thread 3 hit Hardware watchpoint 7: *0xffff0000ebfc5410 Old value = 0
> > > New value = -211812350 kasan_set_track (stack=<optimized out>,
> > > track=<optimized out>) at ./arch/arm64/include/asm/current.h:21
> > > 21 return (struct task_struct *)sp_el0;
> > >
> > > # bt
> > > kasan_set_track
> > > kasan_save_track
> > > kasan_save_free_info
> > > poison_slab_object
> > > __kasan_mempool_poison_object
> > > kasan_mempool_poison_object
> > > io_alloc_cache_put
> > > io_netmsg_recycle
> > > io_req_msg_cleanup
> > > io_connect
> > > io_issue_sqe
> > > io_queue_sqe
> > > io_req_task_submit
> > > ...
> > >
> > >
> > > It's a bit strange. It was modified by KASAN. I can't understand this.
> > > Maybe I missed something? Please let me know. Thanks.
> >
> > Looks like KASAN with the extra info ends up writing to io_async_msghdr-
> > >free_iov somehow. No idea... For the test case in question, ->free_iov should
> > be NULL when initially allocated, and the io_uring code isn't storing to it. Yet
> > it's non-NULL when you later go and free it, after calling
> > kasan_mempool_poison_object().
>
> I also think so and would Juntong and Ryabinin or others KASAN developers be interested
> In this problem?
>
Hi, thanks for reporting.
KASAN stores some info about freed slab object in the object itself
until it is reallocated or the slab page is released.
And since the b556a462eb8d ("kasan: save free stack traces for slab
mempools") we do the same thing in kasan_mempool_poison_object().
In the most use cases this wasn't the problem, because callers expect
uninitialized objects from mempool.
However, this isn't the case for io_alloc_cache. AFAICS io_uring code
expects that io_alloc_cache_put/get leaves objects unmodified.
So I'm thinking we'd need to add some parameter to the
kasan_mempool_poison_object() to avoid modifying objects.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects.
2025-01-22 13:49 ` Andrey Ryabinin
@ 2025-01-22 16:06 ` Andrey Ryabinin
2025-01-25 0:03 ` Andrey Konovalov
2025-01-27 15:03 ` [PATCH v2] " Andrey Ryabinin
0 siblings, 2 replies; 10+ messages in thread
From: Andrey Ryabinin @ 2025-01-22 16:06 UTC (permalink / raw)
To: Andrew Morton
Cc: kasan-dev, io-uring, linux-mm, netdev, linux-kernel, juntong.deng,
lizetao1, Andrey Ryabinin, stable, Alexander Potapenko,
Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Jens Axboe,
Pavel Begunkov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Running the testcase liburing/accept-reust.t with CONFIG_KASAN=y and
CONFIG_KASAN_EXTRA_INFO=y leads to the following crash:
Unable to handle kernel paging request at virtual address 00000c6455008008
...
pc : __kasan_mempool_unpoison_object+0x38/0x170
lr : io_netmsg_cache_free+0x8c/0x180
...
Call trace:
__kasan_mempool_unpoison_object+0x38/0x170 (P)
io_netmsg_cache_free+0x8c/0x180
io_ring_exit_work+0xd4c/0x13a0
process_one_work+0x52c/0x1000
worker_thread+0x830/0xdc0
kthread+0x2bc/0x348
ret_from_fork+0x10/0x20
Since the commit b556a462eb8d ("kasan: save free stack traces for slab mempools")
kasan_mempool_poison_object() stores some info inside an object.
It was expected that the object must be reinitialized after
kasan_mempool_unpoison_object() call, and this is what happens in the
most of use cases.
However io_uring code expects that io_alloc_cache_put/get doesn't modify
the object, so kasan_mempool_poison_object() end up corrupting it leading
to crash later.
Add @notrack argument to kasan_mempool_poison_object() call to tell
KASAN to avoid storing info in objects for io_uring use case.
Reported-by: lizetao <[email protected]>
Closes: https://lkml.kernel.org/r/[email protected]
Fixes: b556a462eb8d ("kasan: save free stack traces for slab mempools")
Cc: [email protected]
Cc: Alexander Potapenko <[email protected]>
Cc: Andrey Konovalov <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Vincenzo Frascino <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Pavel Begunkov <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Simon Horman <[email protected]>
Signed-off-by: Andrey Ryabinin <[email protected]>
---
include/linux/kasan.h | 13 +++++++------
io_uring/alloc_cache.h | 2 +-
io_uring/net.c | 2 +-
io_uring/rw.c | 2 +-
mm/kasan/common.c | 11 ++++++-----
mm/mempool.c | 2 +-
net/core/skbuff.c | 2 +-
7 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 890011071f2b..4d0bf4af399d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -328,18 +328,19 @@ static __always_inline void kasan_mempool_unpoison_pages(struct page *page,
__kasan_mempool_unpoison_pages(page, order, _RET_IP_);
}
-bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
+bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip);
/**
* kasan_mempool_poison_object - Check and poison a mempool slab allocation.
* @ptr: Pointer to the slab allocation.
+ * @notrack: Don't record stack trace of this call in the object.
*
* This function is intended for kernel subsystems that cache slab allocations
* to reuse them instead of freeing them back to the slab allocator (e.g.
* mempool).
*
* This function poisons a slab allocation and saves a free stack trace for it
- * without initializing the allocation's memory and without putting it into the
- * quarantine (for the Generic mode).
+ * (if @notrack == false) without initializing the allocation's memory and
+ * without putting it into the quarantine (for the Generic mode).
*
* This function also performs checks to detect double-free and invalid-free
* bugs and reports them. The caller can use the return value of this function
@@ -354,10 +355,10 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
*
* Return: true if the allocation can be safely reused; false otherwise.
*/
-static __always_inline bool kasan_mempool_poison_object(void *ptr)
+static __always_inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
{
if (kasan_enabled())
- return __kasan_mempool_poison_object(ptr, _RET_IP_);
+ return __kasan_mempool_poison_object(ptr, notrack, _RET_IP_);
return true;
}
@@ -456,7 +457,7 @@ static inline bool kasan_mempool_poison_pages(struct page *page, unsigned int or
return true;
}
static inline void kasan_mempool_unpoison_pages(struct page *page, unsigned int order) {}
-static inline bool kasan_mempool_poison_object(void *ptr)
+static inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
{
return true;
}
diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index a3a8cfec32ce..dd508dddea33 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -10,7 +10,7 @@ static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
void *entry)
{
if (cache->nr_cached < cache->max_cached) {
- if (!kasan_mempool_poison_object(entry))
+ if (!kasan_mempool_poison_object(entry, true))
return false;
cache->entries[cache->nr_cached++] = entry;
return true;
diff --git a/io_uring/net.c b/io_uring/net.c
index 85f55fbc25c9..a954e37c7fd3 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -149,7 +149,7 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
iov = hdr->free_iov;
if (io_alloc_cache_put(&req->ctx->netmsg_cache, hdr)) {
if (iov)
- kasan_mempool_poison_object(iov);
+ kasan_mempool_poison_object(iov, true);
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
}
diff --git a/io_uring/rw.c b/io_uring/rw.c
index a9a2733be842..cba475003ba7 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -167,7 +167,7 @@ static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
iov = rw->free_iovec;
if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
if (iov)
- kasan_mempool_poison_object(iov);
+ kasan_mempool_poison_object(iov, true);
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
}
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index ed4873e18c75..e7b54aa9494e 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -230,7 +230,8 @@ static bool check_slab_allocation(struct kmem_cache *cache, void *object,
}
static inline void poison_slab_object(struct kmem_cache *cache, void *object,
- bool init, bool still_accessible)
+ bool init, bool still_accessible,
+ bool notrack)
{
void *tagged_object = object;
@@ -243,7 +244,7 @@ static inline void poison_slab_object(struct kmem_cache *cache, void *object,
kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZE),
KASAN_SLAB_FREE, init);
- if (kasan_stack_collection_enabled())
+ if (kasan_stack_collection_enabled() && !notrack)
kasan_save_free_info(cache, tagged_object);
}
@@ -261,7 +262,7 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init,
if (!kasan_arch_is_ready() || is_kfence_address(object))
return false;
- poison_slab_object(cache, object, init, still_accessible);
+ poison_slab_object(cache, object, init, still_accessible, true);
/*
* If the object is put into quarantine, do not let slab put the object
@@ -495,7 +496,7 @@ void __kasan_mempool_unpoison_pages(struct page *page, unsigned int order,
__kasan_unpoison_pages(page, order, false);
}
-bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
+bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip)
{
struct folio *folio = virt_to_folio(ptr);
struct slab *slab;
@@ -519,7 +520,7 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
if (check_slab_allocation(slab->slab_cache, ptr, ip))
return false;
- poison_slab_object(slab->slab_cache, ptr, false, false);
+ poison_slab_object(slab->slab_cache, ptr, false, false, notrack);
return true;
}
diff --git a/mm/mempool.c b/mm/mempool.c
index 3223337135d0..283df5d2b995 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -115,7 +115,7 @@ static inline void poison_element(mempool_t *pool, void *element)
static __always_inline bool kasan_poison_element(mempool_t *pool, void *element)
{
if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
- return kasan_mempool_poison_object(element);
+ return kasan_mempool_poison_object(element, false);
else if (pool->alloc == mempool_alloc_pages)
return kasan_mempool_poison_pages(element,
(unsigned long)pool->pool_data);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a441613a1e6c..c9f58a698bb7 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1457,7 +1457,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
u32 i;
- if (!kasan_mempool_poison_object(skb))
+ if (!kasan_mempool_poison_object(skb, false))
return;
local_lock_nested_bh(&napi_alloc_cache.bh_lock);
--
2.45.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects.
2025-01-22 16:06 ` [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects Andrey Ryabinin
@ 2025-01-25 0:03 ` Andrey Konovalov
2025-01-27 13:35 ` Andrey Ryabinin
2025-01-27 15:03 ` [PATCH v2] " Andrey Ryabinin
1 sibling, 1 reply; 10+ messages in thread
From: Andrey Konovalov @ 2025-01-25 0:03 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Andrew Morton, kasan-dev, io-uring, linux-mm, netdev,
linux-kernel, juntong.deng, lizetao1, stable, Alexander Potapenko,
Dmitry Vyukov, Vincenzo Frascino, Jens Axboe, Pavel Begunkov,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
On Wed, Jan 22, 2025 at 5:07 PM Andrey Ryabinin <[email protected]> wrote:
>
> Running the testcase liburing/accept-reust.t with CONFIG_KASAN=y and
> CONFIG_KASAN_EXTRA_INFO=y leads to the following crash:
>
> Unable to handle kernel paging request at virtual address 00000c6455008008
> ...
> pc : __kasan_mempool_unpoison_object+0x38/0x170
> lr : io_netmsg_cache_free+0x8c/0x180
> ...
> Call trace:
> __kasan_mempool_unpoison_object+0x38/0x170 (P)
> io_netmsg_cache_free+0x8c/0x180
> io_ring_exit_work+0xd4c/0x13a0
> process_one_work+0x52c/0x1000
> worker_thread+0x830/0xdc0
> kthread+0x2bc/0x348
> ret_from_fork+0x10/0x20
>
> Since the commit b556a462eb8d ("kasan: save free stack traces for slab mempools")
> kasan_mempool_poison_object() stores some info inside an object.
> It was expected that the object must be reinitialized after
> kasan_mempool_unpoison_object() call, and this is what happens in the
> most of use cases.
>
> However io_uring code expects that io_alloc_cache_put/get doesn't modify
> the object, so kasan_mempool_poison_object() end up corrupting it leading
> to crash later.
>
> Add @notrack argument to kasan_mempool_poison_object() call to tell
> KASAN to avoid storing info in objects for io_uring use case.
>
> Reported-by: lizetao <[email protected]>
> Closes: https://lkml.kernel.org/r/[email protected]
> Fixes: b556a462eb8d ("kasan: save free stack traces for slab mempools")
> Cc: [email protected]
> Cc: Alexander Potapenko <[email protected]>
> Cc: Andrey Konovalov <[email protected]>
> Cc: Dmitry Vyukov <[email protected]>
> Cc: Vincenzo Frascino <[email protected]>
> Cc: Jens Axboe <[email protected]>
> Cc: Pavel Begunkov <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: Simon Horman <[email protected]>
> Signed-off-by: Andrey Ryabinin <[email protected]>
> ---
> include/linux/kasan.h | 13 +++++++------
> io_uring/alloc_cache.h | 2 +-
> io_uring/net.c | 2 +-
> io_uring/rw.c | 2 +-
> mm/kasan/common.c | 11 ++++++-----
> mm/mempool.c | 2 +-
> net/core/skbuff.c | 2 +-
> 7 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index 890011071f2b..4d0bf4af399d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -328,18 +328,19 @@ static __always_inline void kasan_mempool_unpoison_pages(struct page *page,
> __kasan_mempool_unpoison_pages(page, order, _RET_IP_);
> }
>
> -bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
> +bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip);
> /**
> * kasan_mempool_poison_object - Check and poison a mempool slab allocation.
> * @ptr: Pointer to the slab allocation.
> + * @notrack: Don't record stack trace of this call in the object.
> *
> * This function is intended for kernel subsystems that cache slab allocations
> * to reuse them instead of freeing them back to the slab allocator (e.g.
> * mempool).
> *
> * This function poisons a slab allocation and saves a free stack trace for it
> - * without initializing the allocation's memory and without putting it into the
> - * quarantine (for the Generic mode).
> + * (if @notrack == false) without initializing the allocation's memory and
> + * without putting it into the quarantine (for the Generic mode).
> *
> * This function also performs checks to detect double-free and invalid-free
> * bugs and reports them. The caller can use the return value of this function
> @@ -354,10 +355,10 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
> *
> * Return: true if the allocation can be safely reused; false otherwise.
> */
> -static __always_inline bool kasan_mempool_poison_object(void *ptr)
> +static __always_inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
> {
> if (kasan_enabled())
> - return __kasan_mempool_poison_object(ptr, _RET_IP_);
> + return __kasan_mempool_poison_object(ptr, notrack, _RET_IP_);
> return true;
> }
>
> @@ -456,7 +457,7 @@ static inline bool kasan_mempool_poison_pages(struct page *page, unsigned int or
> return true;
> }
> static inline void kasan_mempool_unpoison_pages(struct page *page, unsigned int order) {}
> -static inline bool kasan_mempool_poison_object(void *ptr)
> +static inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
> {
> return true;
> }
> diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
> index a3a8cfec32ce..dd508dddea33 100644
> --- a/io_uring/alloc_cache.h
> +++ b/io_uring/alloc_cache.h
> @@ -10,7 +10,7 @@ static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
> void *entry)
> {
> if (cache->nr_cached < cache->max_cached) {
> - if (!kasan_mempool_poison_object(entry))
> + if (!kasan_mempool_poison_object(entry, true))
> return false;
> cache->entries[cache->nr_cached++] = entry;
> return true;
> diff --git a/io_uring/net.c b/io_uring/net.c
> index 85f55fbc25c9..a954e37c7fd3 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -149,7 +149,7 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
> iov = hdr->free_iov;
> if (io_alloc_cache_put(&req->ctx->netmsg_cache, hdr)) {
> if (iov)
> - kasan_mempool_poison_object(iov);
> + kasan_mempool_poison_object(iov, true);
> req->async_data = NULL;
> req->flags &= ~REQ_F_ASYNC_DATA;
> }
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index a9a2733be842..cba475003ba7 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -167,7 +167,7 @@ static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
> iov = rw->free_iovec;
> if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
> if (iov)
> - kasan_mempool_poison_object(iov);
> + kasan_mempool_poison_object(iov, true);
> req->async_data = NULL;
> req->flags &= ~REQ_F_ASYNC_DATA;
> }
> diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> index ed4873e18c75..e7b54aa9494e 100644
> --- a/mm/kasan/common.c
> +++ b/mm/kasan/common.c
> @@ -230,7 +230,8 @@ static bool check_slab_allocation(struct kmem_cache *cache, void *object,
> }
>
> static inline void poison_slab_object(struct kmem_cache *cache, void *object,
> - bool init, bool still_accessible)
> + bool init, bool still_accessible,
> + bool notrack)
> {
> void *tagged_object = object;
>
> @@ -243,7 +244,7 @@ static inline void poison_slab_object(struct kmem_cache *cache, void *object,
> kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZE),
> KASAN_SLAB_FREE, init);
>
> - if (kasan_stack_collection_enabled())
> + if (kasan_stack_collection_enabled() && !notrack)
> kasan_save_free_info(cache, tagged_object);
> }
>
> @@ -261,7 +262,7 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init,
> if (!kasan_arch_is_ready() || is_kfence_address(object))
> return false;
>
> - poison_slab_object(cache, object, init, still_accessible);
> + poison_slab_object(cache, object, init, still_accessible, true);
Should notrack be false here?
>
> /*
> * If the object is put into quarantine, do not let slab put the object
> @@ -495,7 +496,7 @@ void __kasan_mempool_unpoison_pages(struct page *page, unsigned int order,
> __kasan_unpoison_pages(page, order, false);
> }
>
> -bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
> +bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip)
> {
> struct folio *folio = virt_to_folio(ptr);
> struct slab *slab;
> @@ -519,7 +520,7 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
> if (check_slab_allocation(slab->slab_cache, ptr, ip))
> return false;
>
> - poison_slab_object(slab->slab_cache, ptr, false, false);
> + poison_slab_object(slab->slab_cache, ptr, false, false, notrack);
> return true;
> }
>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index 3223337135d0..283df5d2b995 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -115,7 +115,7 @@ static inline void poison_element(mempool_t *pool, void *element)
> static __always_inline bool kasan_poison_element(mempool_t *pool, void *element)
> {
> if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> - return kasan_mempool_poison_object(element);
> + return kasan_mempool_poison_object(element, false);
> else if (pool->alloc == mempool_alloc_pages)
> return kasan_mempool_poison_pages(element,
> (unsigned long)pool->pool_data);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index a441613a1e6c..c9f58a698bb7 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1457,7 +1457,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
> struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
> u32 i;
>
> - if (!kasan_mempool_poison_object(skb))
> + if (!kasan_mempool_poison_object(skb, false))
> return;
>
> local_lock_nested_bh(&napi_alloc_cache.bh_lock);
> --
> 2.45.3
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects.
2025-01-25 0:03 ` Andrey Konovalov
@ 2025-01-27 13:35 ` Andrey Ryabinin
0 siblings, 0 replies; 10+ messages in thread
From: Andrey Ryabinin @ 2025-01-27 13:35 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Andrew Morton, kasan-dev, io-uring, linux-mm, netdev,
linux-kernel, juntong.deng, lizetao1, stable, Alexander Potapenko,
Dmitry Vyukov, Vincenzo Frascino, Jens Axboe, Pavel Begunkov,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
On Sat, Jan 25, 2025 at 1:03 AM Andrey Konovalov <[email protected]> wrote:
>
> On Wed, Jan 22, 2025 at 5:07 PM Andrey Ryabinin <[email protected]> wrote:
> > @@ -261,7 +262,7 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init,
> > if (!kasan_arch_is_ready() || is_kfence_address(object))
> > return false;
> >
> > - poison_slab_object(cache, object, init, still_accessible);
> > + poison_slab_object(cache, object, init, still_accessible, true);
>
> Should notrack be false here?
>
Yep.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] kasan, mempool: don't store free stacktrace in io_alloc_cache objects.
2025-01-22 16:06 ` [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects Andrey Ryabinin
2025-01-25 0:03 ` Andrey Konovalov
@ 2025-01-27 15:03 ` Andrey Ryabinin
2025-01-28 1:03 ` Andrey Konovalov
2025-01-30 16:02 ` Jens Axboe
1 sibling, 2 replies; 10+ messages in thread
From: Andrey Ryabinin @ 2025-01-27 15:03 UTC (permalink / raw)
To: Andrew Morton
Cc: kasan-dev, io-uring, linux-mm, netdev, linux-kernel, juntong.deng,
lizetao1, Andrey Ryabinin, stable, Alexander Potapenko,
Andrey Konovalov, Dmitry Vyukov, Vincenzo Frascino, Jens Axboe,
Pavel Begunkov, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Simon Horman
Running the testcase liburing/accept-reust.t with CONFIG_KASAN=y and
CONFIG_KASAN_EXTRA_INFO=y leads to the following crash:
Unable to handle kernel paging request at virtual address 00000c6455008008
...
pc : __kasan_mempool_unpoison_object+0x38/0x170
lr : io_netmsg_cache_free+0x8c/0x180
...
Call trace:
__kasan_mempool_unpoison_object+0x38/0x170 (P)
io_netmsg_cache_free+0x8c/0x180
io_ring_exit_work+0xd4c/0x13a0
process_one_work+0x52c/0x1000
worker_thread+0x830/0xdc0
kthread+0x2bc/0x348
ret_from_fork+0x10/0x20
Since the commit b556a462eb8d ("kasan: save free stack traces for slab mempools")
kasan_mempool_poison_object() stores some info inside an object.
It was expected that the object must be reinitialized after
kasan_mempool_unpoison_object() call, and this is what happens in the
most of use cases.
However io_uring code expects that io_alloc_cache_put/get doesn't modify
the object, so kasan_mempool_poison_object() end up corrupting it leading
to crash later.
Add @notrack argument to kasan_mempool_poison_object() call to tell
KASAN to avoid storing info in objects for io_uring use case.
Reported-by: lizetao <[email protected]>
Closes: https://lkml.kernel.org/r/[email protected]
Fixes: b556a462eb8d ("kasan: save free stack traces for slab mempools")
Cc: [email protected]
Cc: Alexander Potapenko <[email protected]>
Cc: Andrey Konovalov <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Vincenzo Frascino <[email protected]>
Cc: Jens Axboe <[email protected]>
Cc: Pavel Begunkov <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Eric Dumazet <[email protected]>
Cc: Jakub Kicinski <[email protected]>
Cc: Paolo Abeni <[email protected]>
Cc: Simon Horman <[email protected]>
Signed-off-by: Andrey Ryabinin <[email protected]>
---
- Changes since v1:
s/true/false @notrack in __kasan_slab_free() per @andreyknvl
include/linux/kasan.h | 13 +++++++------
io_uring/alloc_cache.h | 2 +-
io_uring/net.c | 2 +-
io_uring/rw.c | 2 +-
mm/kasan/common.c | 11 ++++++-----
mm/mempool.c | 2 +-
net/core/skbuff.c | 2 +-
7 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index 890011071f2b..4d0bf4af399d 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -328,18 +328,19 @@ static __always_inline void kasan_mempool_unpoison_pages(struct page *page,
__kasan_mempool_unpoison_pages(page, order, _RET_IP_);
}
-bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
+bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip);
/**
* kasan_mempool_poison_object - Check and poison a mempool slab allocation.
* @ptr: Pointer to the slab allocation.
+ * @notrack: Don't record stack trace of this call in the object.
*
* This function is intended for kernel subsystems that cache slab allocations
* to reuse them instead of freeing them back to the slab allocator (e.g.
* mempool).
*
* This function poisons a slab allocation and saves a free stack trace for it
- * without initializing the allocation's memory and without putting it into the
- * quarantine (for the Generic mode).
+ * (if @notrack == false) without initializing the allocation's memory and
+ * without putting it into the quarantine (for the Generic mode).
*
* This function also performs checks to detect double-free and invalid-free
* bugs and reports them. The caller can use the return value of this function
@@ -354,10 +355,10 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
*
* Return: true if the allocation can be safely reused; false otherwise.
*/
-static __always_inline bool kasan_mempool_poison_object(void *ptr)
+static __always_inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
{
if (kasan_enabled())
- return __kasan_mempool_poison_object(ptr, _RET_IP_);
+ return __kasan_mempool_poison_object(ptr, notrack, _RET_IP_);
return true;
}
@@ -456,7 +457,7 @@ static inline bool kasan_mempool_poison_pages(struct page *page, unsigned int or
return true;
}
static inline void kasan_mempool_unpoison_pages(struct page *page, unsigned int order) {}
-static inline bool kasan_mempool_poison_object(void *ptr)
+static inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
{
return true;
}
diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
index a3a8cfec32ce..dd508dddea33 100644
--- a/io_uring/alloc_cache.h
+++ b/io_uring/alloc_cache.h
@@ -10,7 +10,7 @@ static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
void *entry)
{
if (cache->nr_cached < cache->max_cached) {
- if (!kasan_mempool_poison_object(entry))
+ if (!kasan_mempool_poison_object(entry, true))
return false;
cache->entries[cache->nr_cached++] = entry;
return true;
diff --git a/io_uring/net.c b/io_uring/net.c
index 85f55fbc25c9..a954e37c7fd3 100644
--- a/io_uring/net.c
+++ b/io_uring/net.c
@@ -149,7 +149,7 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
iov = hdr->free_iov;
if (io_alloc_cache_put(&req->ctx->netmsg_cache, hdr)) {
if (iov)
- kasan_mempool_poison_object(iov);
+ kasan_mempool_poison_object(iov, true);
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
}
diff --git a/io_uring/rw.c b/io_uring/rw.c
index a9a2733be842..cba475003ba7 100644
--- a/io_uring/rw.c
+++ b/io_uring/rw.c
@@ -167,7 +167,7 @@ static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
iov = rw->free_iovec;
if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
if (iov)
- kasan_mempool_poison_object(iov);
+ kasan_mempool_poison_object(iov, true);
req->async_data = NULL;
req->flags &= ~REQ_F_ASYNC_DATA;
}
diff --git a/mm/kasan/common.c b/mm/kasan/common.c
index ed4873e18c75..f08752dcd50b 100644
--- a/mm/kasan/common.c
+++ b/mm/kasan/common.c
@@ -230,7 +230,8 @@ static bool check_slab_allocation(struct kmem_cache *cache, void *object,
}
static inline void poison_slab_object(struct kmem_cache *cache, void *object,
- bool init, bool still_accessible)
+ bool init, bool still_accessible,
+ bool notrack)
{
void *tagged_object = object;
@@ -243,7 +244,7 @@ static inline void poison_slab_object(struct kmem_cache *cache, void *object,
kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZE),
KASAN_SLAB_FREE, init);
- if (kasan_stack_collection_enabled())
+ if (kasan_stack_collection_enabled() && !notrack)
kasan_save_free_info(cache, tagged_object);
}
@@ -261,7 +262,7 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init,
if (!kasan_arch_is_ready() || is_kfence_address(object))
return false;
- poison_slab_object(cache, object, init, still_accessible);
+ poison_slab_object(cache, object, init, still_accessible, false);
/*
* If the object is put into quarantine, do not let slab put the object
@@ -495,7 +496,7 @@ void __kasan_mempool_unpoison_pages(struct page *page, unsigned int order,
__kasan_unpoison_pages(page, order, false);
}
-bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
+bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip)
{
struct folio *folio = virt_to_folio(ptr);
struct slab *slab;
@@ -519,7 +520,7 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
if (check_slab_allocation(slab->slab_cache, ptr, ip))
return false;
- poison_slab_object(slab->slab_cache, ptr, false, false);
+ poison_slab_object(slab->slab_cache, ptr, false, false, notrack);
return true;
}
diff --git a/mm/mempool.c b/mm/mempool.c
index 3223337135d0..283df5d2b995 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -115,7 +115,7 @@ static inline void poison_element(mempool_t *pool, void *element)
static __always_inline bool kasan_poison_element(mempool_t *pool, void *element)
{
if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
- return kasan_mempool_poison_object(element);
+ return kasan_mempool_poison_object(element, false);
else if (pool->alloc == mempool_alloc_pages)
return kasan_mempool_poison_pages(element,
(unsigned long)pool->pool_data);
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index a441613a1e6c..c9f58a698bb7 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -1457,7 +1457,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
u32 i;
- if (!kasan_mempool_poison_object(skb))
+ if (!kasan_mempool_poison_object(skb, false))
return;
local_lock_nested_bh(&napi_alloc_cache.bh_lock);
--
2.45.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2] kasan, mempool: don't store free stacktrace in io_alloc_cache objects.
2025-01-27 15:03 ` [PATCH v2] " Andrey Ryabinin
@ 2025-01-28 1:03 ` Andrey Konovalov
2025-01-30 16:02 ` Jens Axboe
1 sibling, 0 replies; 10+ messages in thread
From: Andrey Konovalov @ 2025-01-28 1:03 UTC (permalink / raw)
To: Andrey Ryabinin
Cc: Andrew Morton, kasan-dev, io-uring, linux-mm, netdev,
linux-kernel, juntong.deng, lizetao1, stable, Alexander Potapenko,
Dmitry Vyukov, Vincenzo Frascino, Jens Axboe, Pavel Begunkov,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman
On Mon, Jan 27, 2025 at 4:05 PM Andrey Ryabinin <[email protected]> wrote:
>
> Running the testcase liburing/accept-reust.t with CONFIG_KASAN=y and
> CONFIG_KASAN_EXTRA_INFO=y leads to the following crash:
>
> Unable to handle kernel paging request at virtual address 00000c6455008008
> ...
> pc : __kasan_mempool_unpoison_object+0x38/0x170
> lr : io_netmsg_cache_free+0x8c/0x180
> ...
> Call trace:
> __kasan_mempool_unpoison_object+0x38/0x170 (P)
> io_netmsg_cache_free+0x8c/0x180
> io_ring_exit_work+0xd4c/0x13a0
> process_one_work+0x52c/0x1000
> worker_thread+0x830/0xdc0
> kthread+0x2bc/0x348
> ret_from_fork+0x10/0x20
>
> Since the commit b556a462eb8d ("kasan: save free stack traces for slab mempools")
> kasan_mempool_poison_object() stores some info inside an object.
> It was expected that the object must be reinitialized after
> kasan_mempool_unpoison_object() call, and this is what happens in the
> most of use cases.
>
> However io_uring code expects that io_alloc_cache_put/get doesn't modify
> the object, so kasan_mempool_poison_object() end up corrupting it leading
> to crash later.
>
> Add @notrack argument to kasan_mempool_poison_object() call to tell
> KASAN to avoid storing info in objects for io_uring use case.
>
> Reported-by: lizetao <[email protected]>
> Closes: https://lkml.kernel.org/r/[email protected]
> Fixes: b556a462eb8d ("kasan: save free stack traces for slab mempools")
> Cc: [email protected]
> Cc: Alexander Potapenko <[email protected]>
> Cc: Andrey Konovalov <[email protected]>
> Cc: Dmitry Vyukov <[email protected]>
> Cc: Vincenzo Frascino <[email protected]>
> Cc: Jens Axboe <[email protected]>
> Cc: Pavel Begunkov <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Eric Dumazet <[email protected]>
> Cc: Jakub Kicinski <[email protected]>
> Cc: Paolo Abeni <[email protected]>
> Cc: Simon Horman <[email protected]>
> Signed-off-by: Andrey Ryabinin <[email protected]>
> ---
> - Changes since v1:
> s/true/false @notrack in __kasan_slab_free() per @andreyknvl
>
> include/linux/kasan.h | 13 +++++++------
> io_uring/alloc_cache.h | 2 +-
> io_uring/net.c | 2 +-
> io_uring/rw.c | 2 +-
> mm/kasan/common.c | 11 ++++++-----
> mm/mempool.c | 2 +-
> net/core/skbuff.c | 2 +-
> 7 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/kasan.h b/include/linux/kasan.h
> index 890011071f2b..4d0bf4af399d 100644
> --- a/include/linux/kasan.h
> +++ b/include/linux/kasan.h
> @@ -328,18 +328,19 @@ static __always_inline void kasan_mempool_unpoison_pages(struct page *page,
> __kasan_mempool_unpoison_pages(page, order, _RET_IP_);
> }
>
> -bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
> +bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip);
> /**
> * kasan_mempool_poison_object - Check and poison a mempool slab allocation.
> * @ptr: Pointer to the slab allocation.
> + * @notrack: Don't record stack trace of this call in the object.
> *
> * This function is intended for kernel subsystems that cache slab allocations
> * to reuse them instead of freeing them back to the slab allocator (e.g.
> * mempool).
> *
> * This function poisons a slab allocation and saves a free stack trace for it
> - * without initializing the allocation's memory and without putting it into the
> - * quarantine (for the Generic mode).
> + * (if @notrack == false) without initializing the allocation's memory and
> + * without putting it into the quarantine (for the Generic mode).
> *
> * This function also performs checks to detect double-free and invalid-free
> * bugs and reports them. The caller can use the return value of this function
> @@ -354,10 +355,10 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip);
> *
> * Return: true if the allocation can be safely reused; false otherwise.
> */
> -static __always_inline bool kasan_mempool_poison_object(void *ptr)
> +static __always_inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
> {
> if (kasan_enabled())
> - return __kasan_mempool_poison_object(ptr, _RET_IP_);
> + return __kasan_mempool_poison_object(ptr, notrack, _RET_IP_);
> return true;
> }
>
> @@ -456,7 +457,7 @@ static inline bool kasan_mempool_poison_pages(struct page *page, unsigned int or
> return true;
> }
> static inline void kasan_mempool_unpoison_pages(struct page *page, unsigned int order) {}
> -static inline bool kasan_mempool_poison_object(void *ptr)
> +static inline bool kasan_mempool_poison_object(void *ptr, bool notrack)
> {
> return true;
> }
> diff --git a/io_uring/alloc_cache.h b/io_uring/alloc_cache.h
> index a3a8cfec32ce..dd508dddea33 100644
> --- a/io_uring/alloc_cache.h
> +++ b/io_uring/alloc_cache.h
> @@ -10,7 +10,7 @@ static inline bool io_alloc_cache_put(struct io_alloc_cache *cache,
> void *entry)
> {
> if (cache->nr_cached < cache->max_cached) {
> - if (!kasan_mempool_poison_object(entry))
> + if (!kasan_mempool_poison_object(entry, true))
> return false;
> cache->entries[cache->nr_cached++] = entry;
> return true;
> diff --git a/io_uring/net.c b/io_uring/net.c
> index 85f55fbc25c9..a954e37c7fd3 100644
> --- a/io_uring/net.c
> +++ b/io_uring/net.c
> @@ -149,7 +149,7 @@ static void io_netmsg_recycle(struct io_kiocb *req, unsigned int issue_flags)
> iov = hdr->free_iov;
> if (io_alloc_cache_put(&req->ctx->netmsg_cache, hdr)) {
> if (iov)
> - kasan_mempool_poison_object(iov);
> + kasan_mempool_poison_object(iov, true);
> req->async_data = NULL;
> req->flags &= ~REQ_F_ASYNC_DATA;
> }
> diff --git a/io_uring/rw.c b/io_uring/rw.c
> index a9a2733be842..cba475003ba7 100644
> --- a/io_uring/rw.c
> +++ b/io_uring/rw.c
> @@ -167,7 +167,7 @@ static void io_rw_recycle(struct io_kiocb *req, unsigned int issue_flags)
> iov = rw->free_iovec;
> if (io_alloc_cache_put(&req->ctx->rw_cache, rw)) {
> if (iov)
> - kasan_mempool_poison_object(iov);
> + kasan_mempool_poison_object(iov, true);
> req->async_data = NULL;
> req->flags &= ~REQ_F_ASYNC_DATA;
> }
> diff --git a/mm/kasan/common.c b/mm/kasan/common.c
> index ed4873e18c75..f08752dcd50b 100644
> --- a/mm/kasan/common.c
> +++ b/mm/kasan/common.c
> @@ -230,7 +230,8 @@ static bool check_slab_allocation(struct kmem_cache *cache, void *object,
> }
>
> static inline void poison_slab_object(struct kmem_cache *cache, void *object,
> - bool init, bool still_accessible)
> + bool init, bool still_accessible,
> + bool notrack)
> {
> void *tagged_object = object;
>
> @@ -243,7 +244,7 @@ static inline void poison_slab_object(struct kmem_cache *cache, void *object,
> kasan_poison(object, round_up(cache->object_size, KASAN_GRANULE_SIZE),
> KASAN_SLAB_FREE, init);
>
> - if (kasan_stack_collection_enabled())
> + if (kasan_stack_collection_enabled() && !notrack)
> kasan_save_free_info(cache, tagged_object);
> }
>
> @@ -261,7 +262,7 @@ bool __kasan_slab_free(struct kmem_cache *cache, void *object, bool init,
> if (!kasan_arch_is_ready() || is_kfence_address(object))
> return false;
>
> - poison_slab_object(cache, object, init, still_accessible);
> + poison_slab_object(cache, object, init, still_accessible, false);
>
> /*
> * If the object is put into quarantine, do not let slab put the object
> @@ -495,7 +496,7 @@ void __kasan_mempool_unpoison_pages(struct page *page, unsigned int order,
> __kasan_unpoison_pages(page, order, false);
> }
>
> -bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
> +bool __kasan_mempool_poison_object(void *ptr, bool notrack, unsigned long ip)
> {
> struct folio *folio = virt_to_folio(ptr);
> struct slab *slab;
> @@ -519,7 +520,7 @@ bool __kasan_mempool_poison_object(void *ptr, unsigned long ip)
> if (check_slab_allocation(slab->slab_cache, ptr, ip))
> return false;
>
> - poison_slab_object(slab->slab_cache, ptr, false, false);
> + poison_slab_object(slab->slab_cache, ptr, false, false, notrack);
> return true;
> }
>
> diff --git a/mm/mempool.c b/mm/mempool.c
> index 3223337135d0..283df5d2b995 100644
> --- a/mm/mempool.c
> +++ b/mm/mempool.c
> @@ -115,7 +115,7 @@ static inline void poison_element(mempool_t *pool, void *element)
> static __always_inline bool kasan_poison_element(mempool_t *pool, void *element)
> {
> if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
> - return kasan_mempool_poison_object(element);
> + return kasan_mempool_poison_object(element, false);
> else if (pool->alloc == mempool_alloc_pages)
> return kasan_mempool_poison_pages(element,
> (unsigned long)pool->pool_data);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index a441613a1e6c..c9f58a698bb7 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1457,7 +1457,7 @@ static void napi_skb_cache_put(struct sk_buff *skb)
> struct napi_alloc_cache *nc = this_cpu_ptr(&napi_alloc_cache);
> u32 i;
>
> - if (!kasan_mempool_poison_object(skb))
> + if (!kasan_mempool_poison_object(skb, false))
> return;
>
> local_lock_nested_bh(&napi_alloc_cache.bh_lock);
> --
> 2.45.3
>
Reviewed-by: Andrey Konovalov <[email protected]>
Thank you!
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] kasan, mempool: don't store free stacktrace in io_alloc_cache objects.
2025-01-27 15:03 ` [PATCH v2] " Andrey Ryabinin
2025-01-28 1:03 ` Andrey Konovalov
@ 2025-01-30 16:02 ` Jens Axboe
1 sibling, 0 replies; 10+ messages in thread
From: Jens Axboe @ 2025-01-30 16:02 UTC (permalink / raw)
To: Andrey Ryabinin, Andrew Morton
Cc: kasan-dev, io-uring, linux-mm, netdev, linux-kernel, juntong.deng,
lizetao1, stable, Alexander Potapenko, Andrey Konovalov,
Dmitry Vyukov, Vincenzo Frascino, Pavel Begunkov, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
I don't think we need this with the recent cleanup of the io_uring
struct caching. That should go into 6.14-rc1, it's queued up. So I think
let's defer on this one for now? It'll conflict with those changes too.
--
Jens Axboe
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-01-30 16:02 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-11 14:07 KASAN reported an error while executing accept-reust.t testcase lizetao
2025-01-11 17:13 ` Jens Axboe
2025-01-12 6:45 ` lizetao
2025-01-22 13:49 ` Andrey Ryabinin
2025-01-22 16:06 ` [PATCH] kasan, mempool: don't store free stacktrace in io_alloc_cache objects Andrey Ryabinin
2025-01-25 0:03 ` Andrey Konovalov
2025-01-27 13:35 ` Andrey Ryabinin
2025-01-27 15:03 ` [PATCH v2] " Andrey Ryabinin
2025-01-28 1:03 ` Andrey Konovalov
2025-01-30 16:02 ` Jens Axboe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox