* 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