| Message ID | 20230208142508.3278406-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0b34d68049b09821499b93d50b5a9d7d2ca449f6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: enable usercopy for skb_small_head_cache | expand |
On Wed, Feb 8, 2023 at 9:25 AM Eric Dumazet <edumazet@google.com> wrote: > > syzbot and other bots reported that we have to enable > user copy to/from skb->head. [1] > > We can prevent access to skb_shared_info, which is a nice > improvement over standard kmem_cache. > > Layout of these kmem_cache objects is: > > < SKB_SMALL_HEAD_HEADROOM >< struct skb_shared_info > > > usercopy: Kernel memory overwrite attempt detected to SLUB object 'skbuff_small_head' (offset 32, size 20)! > ------------[ cut here ]------------ > kernel BUG at mm/usercopy.c:102 ! > invalid opcode: 0000 [#1] PREEMPT SMP KASAN > CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.2.0-rc6-syzkaller-01425-gcb6b2e11a42d #0 > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023 > RIP: 0010:usercopy_abort+0xbd/0xbf mm/usercopy.c:102 > Code: e8 ee ad ba f7 49 89 d9 4d 89 e8 4c 89 e1 41 56 48 89 ee 48 c7 c7 20 2b 5b 8a ff 74 24 08 41 57 48 8b 54 24 20 e8 7a 17 fe ff <0f> 0b e8 c2 ad ba f7 e8 7d fb 08 f8 48 8b 0c 24 49 89 d8 44 89 ea > RSP: 0000:ffffc90000067a48 EFLAGS: 00010286 > RAX: 000000000000006b RBX: ffffffff8b5b6ea0 RCX: 0000000000000000 > RDX: ffff8881401c0000 RSI: ffffffff8166195c RDI: fffff5200000cf3b > RBP: ffffffff8a5b2a60 R08: 000000000000006b R09: 0000000000000000 > R10: 0000000080000000 R11: 0000000000000000 R12: ffffffff8bf2a925 > R13: ffffffff8a5b29a0 R14: 0000000000000014 R15: ffffffff8a5b2960 > FS: 0000000000000000(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000000 CR3: 000000000c48e000 CR4: 00000000003506e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > __check_heap_object+0xdd/0x110 mm/slub.c:4761 > check_heap_object mm/usercopy.c:196 [inline] > __check_object_size mm/usercopy.c:251 [inline] > __check_object_size+0x1da/0x5a0 mm/usercopy.c:213 > check_object_size include/linux/thread_info.h:199 [inline] > check_copy_size include/linux/thread_info.h:235 [inline] > copy_from_iter include/linux/uio.h:186 [inline] > copy_from_iter_full include/linux/uio.h:194 [inline] > memcpy_from_msg include/linux/skbuff.h:3977 [inline] > qrtr_sendmsg+0x65f/0x970 net/qrtr/af_qrtr.c:965 > sock_sendmsg_nosec net/socket.c:722 [inline] > sock_sendmsg+0xde/0x190 net/socket.c:745 > say_hello+0xf6/0x170 net/qrtr/ns.c:325 > qrtr_ns_init+0x220/0x2b0 net/qrtr/ns.c:804 > qrtr_proto_init+0x59/0x95 net/qrtr/af_qrtr.c:1296 > do_one_initcall+0x141/0x790 init/main.c:1306 > do_initcall_level init/main.c:1379 [inline] > do_initcalls init/main.c:1395 [inline] > do_basic_setup init/main.c:1414 [inline] > kernel_init_freeable+0x6f9/0x782 init/main.c:1634 > kernel_init+0x1e/0x1d0 init/main.c:1522 > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308 > </TASK> > > Fixes: bf9f1baa279f ("net: add dedicated kmem_cache for typical/small skb->head") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Acked-by: Soheil Hassas Yeganeh <soheil@google.com> Thank you for the quick fix! > --- > net/core/skbuff.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/net/core/skbuff.c b/net/core/skbuff.c > index bdb1e015e32b9386139e9ad73acd6efb3c357118..70a6088e832682efccf081fa3e6a97cbdeb747ac 100644 > --- a/net/core/skbuff.c > +++ b/net/core/skbuff.c > @@ -4690,10 +4690,16 @@ void __init skb_init(void) > SLAB_HWCACHE_ALIGN|SLAB_PANIC, > NULL); > #ifdef HAVE_SKB_SMALL_HEAD_CACHE > - skb_small_head_cache = kmem_cache_create("skbuff_small_head", > + /* usercopy should only access first SKB_SMALL_HEAD_HEADROOM bytes. > + * struct skb_shared_info is located at the end of skb->head, > + * and should not be copied to/from user. > + */ > + skb_small_head_cache = kmem_cache_create_usercopy("skbuff_small_head", > SKB_SMALL_HEAD_CACHE_SIZE, > 0, > SLAB_HWCACHE_ALIGN | SLAB_PANIC, > + 0, > + SKB_SMALL_HEAD_HEADROOM, > NULL); > #endif > skb_extensions_init(); > -- > 2.39.1.519.gcb327c4b5f-goog >
On Wed, Feb 08, 2023 at 02:25:08PM +0000, 'Eric Dumazet' via syzkaller wrote: > syzbot and other bots reported that we have to enable > user copy to/from skb->head. [1] > > We can prevent access to skb_shared_info, which is a nice > improvement over standard kmem_cache. > > Layout of these kmem_cache objects is: > > < SKB_SMALL_HEAD_HEADROOM >< struct skb_shared_info > [...] > > Fixes: bf9f1baa279f ("net: add dedicated kmem_cache for typical/small skb->head") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Tested-by: Ido Schimmel <idosch@nvidia.com> Hit this one as well, patch solved the problem. Thanks!
> syzbot and other bots reported that we have to enable > user copy to/from skb->head. [1] > > We can prevent access to skb_shared_info, which is a nice > improvement over standard kmem_cache. > > Layout of these kmem_cache objects is: > > < SKB_SMALL_HEAD_HEADROOM >< struct skb_shared_info > > > usercopy: Kernel memory overwrite attempt detected to SLUB object 'skbuff_small_head' (offset 32, size 20)! > ------------[ cut here ]------------ > kernel BUG at mm/usercopy.c:102 ! [...] LKFT also reported this problem on today's Linux next-20230209. Link: https://lore.kernel.org/linux-next/CA+G9fYs-i-c2KTSA7Ai4ES_ZESY1ZnM=Zuo8P1jN00oed6KHMA@mail.gmail.com Reported-by: Linux Kernel Functional Testing <lkft@linaro.org> > > Fixes: bf9f1baa279f ("net: add dedicated kmem_cache for typical/small skb->head") > Reported-by: syzbot <syzkaller@googlegroups.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Tested-by: Linux Kernel Functional Testing <lkft@linaro.org> Thanks for providing a quick fix. -- Linaro LKFT https://lkft.linaro.org
Hello: This patch was applied to netdev/net-next.git (master) by Jakub Kicinski <kuba@kernel.org>: On Wed, 8 Feb 2023 14:25:08 +0000 you wrote: > syzbot and other bots reported that we have to enable > user copy to/from skb->head. [1] > > We can prevent access to skb_shared_info, which is a nice > improvement over standard kmem_cache. > > Layout of these kmem_cache objects is: > > [...] Here is the summary with links: - [net-next] net: enable usercopy for skb_small_head_cache https://git.kernel.org/netdev/net-next/c/0b34d68049b0 You are awesome, thank you!
diff --git a/net/core/skbuff.c b/net/core/skbuff.c index bdb1e015e32b9386139e9ad73acd6efb3c357118..70a6088e832682efccf081fa3e6a97cbdeb747ac 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -4690,10 +4690,16 @@ void __init skb_init(void) SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL); #ifdef HAVE_SKB_SMALL_HEAD_CACHE - skb_small_head_cache = kmem_cache_create("skbuff_small_head", + /* usercopy should only access first SKB_SMALL_HEAD_HEADROOM bytes. + * struct skb_shared_info is located at the end of skb->head, + * and should not be copied to/from user. + */ + skb_small_head_cache = kmem_cache_create_usercopy("skbuff_small_head", SKB_SMALL_HEAD_CACHE_SIZE, 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC, + 0, + SKB_SMALL_HEAD_HEADROOM, NULL); #endif skb_extensions_init();
syzbot and other bots reported that we have to enable user copy to/from skb->head. [1] We can prevent access to skb_shared_info, which is a nice improvement over standard kmem_cache. Layout of these kmem_cache objects is: < SKB_SMALL_HEAD_HEADROOM >< struct skb_shared_info > usercopy: Kernel memory overwrite attempt detected to SLUB object 'skbuff_small_head' (offset 32, size 20)! ------------[ cut here ]------------ kernel BUG at mm/usercopy.c:102 ! invalid opcode: 0000 [#1] PREEMPT SMP KASAN CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.2.0-rc6-syzkaller-01425-gcb6b2e11a42d #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/12/2023 RIP: 0010:usercopy_abort+0xbd/0xbf mm/usercopy.c:102 Code: e8 ee ad ba f7 49 89 d9 4d 89 e8 4c 89 e1 41 56 48 89 ee 48 c7 c7 20 2b 5b 8a ff 74 24 08 41 57 48 8b 54 24 20 e8 7a 17 fe ff <0f> 0b e8 c2 ad ba f7 e8 7d fb 08 f8 48 8b 0c 24 49 89 d8 44 89 ea RSP: 0000:ffffc90000067a48 EFLAGS: 00010286 RAX: 000000000000006b RBX: ffffffff8b5b6ea0 RCX: 0000000000000000 RDX: ffff8881401c0000 RSI: ffffffff8166195c RDI: fffff5200000cf3b RBP: ffffffff8a5b2a60 R08: 000000000000006b R09: 0000000000000000 R10: 0000000080000000 R11: 0000000000000000 R12: ffffffff8bf2a925 R13: ffffffff8a5b29a0 R14: 0000000000000014 R15: ffffffff8a5b2960 FS: 0000000000000000(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 000000000c48e000 CR4: 00000000003506e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> __check_heap_object+0xdd/0x110 mm/slub.c:4761 check_heap_object mm/usercopy.c:196 [inline] __check_object_size mm/usercopy.c:251 [inline] __check_object_size+0x1da/0x5a0 mm/usercopy.c:213 check_object_size include/linux/thread_info.h:199 [inline] check_copy_size include/linux/thread_info.h:235 [inline] copy_from_iter include/linux/uio.h:186 [inline] copy_from_iter_full include/linux/uio.h:194 [inline] memcpy_from_msg include/linux/skbuff.h:3977 [inline] qrtr_sendmsg+0x65f/0x970 net/qrtr/af_qrtr.c:965 sock_sendmsg_nosec net/socket.c:722 [inline] sock_sendmsg+0xde/0x190 net/socket.c:745 say_hello+0xf6/0x170 net/qrtr/ns.c:325 qrtr_ns_init+0x220/0x2b0 net/qrtr/ns.c:804 qrtr_proto_init+0x59/0x95 net/qrtr/af_qrtr.c:1296 do_one_initcall+0x141/0x790 init/main.c:1306 do_initcall_level init/main.c:1379 [inline] do_initcalls init/main.c:1395 [inline] do_basic_setup init/main.c:1414 [inline] kernel_init_freeable+0x6f9/0x782 init/main.c:1634 kernel_init+0x1e/0x1d0 init/main.c:1522 ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:308 </TASK> Fixes: bf9f1baa279f ("net: add dedicated kmem_cache for typical/small skb->head") Reported-by: syzbot <syzkaller@googlegroups.com> Signed-off-by: Eric Dumazet <edumazet@google.com> --- net/core/skbuff.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)