| Message ID | 20230410012352.3997823-1-william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [net,v2] net: qrtr: Fix an uninit variable access bug in qrtr_tx_resume() | expand |
On Mon, Apr 10, 2023 at 09:23:52AM +0800, Ziyang Xuan wrote: > Syzbot reported a bug as following: > > ===================================================== > BUG: KMSAN: uninit-value in qrtr_tx_resume+0x185/0x1f0 net/qrtr/af_qrtr.c:230 > qrtr_tx_resume+0x185/0x1f0 net/qrtr/af_qrtr.c:230 > qrtr_endpoint_post+0xf85/0x11b0 net/qrtr/af_qrtr.c:519 > qrtr_tun_write_iter+0x270/0x400 net/qrtr/tun.c:108 > call_write_iter include/linux/fs.h:2189 [inline] > aio_write+0x63a/0x950 fs/aio.c:1600 > io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 > __do_sys_io_submit fs/aio.c:2078 [inline] > __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 > __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > Uninit was created at: > slab_post_alloc_hook mm/slab.h:766 [inline] > slab_alloc_node mm/slub.c:3452 [inline] > __kmem_cache_alloc_node+0x71f/0xce0 mm/slub.c:3491 > __do_kmalloc_node mm/slab_common.c:967 [inline] > __kmalloc_node_track_caller+0x114/0x3b0 mm/slab_common.c:988 > kmalloc_reserve net/core/skbuff.c:492 [inline] > __alloc_skb+0x3af/0x8f0 net/core/skbuff.c:565 > __netdev_alloc_skb+0x120/0x7d0 net/core/skbuff.c:630 > qrtr_endpoint_post+0xbd/0x11b0 net/qrtr/af_qrtr.c:446 > qrtr_tun_write_iter+0x270/0x400 net/qrtr/tun.c:108 > call_write_iter include/linux/fs.h:2189 [inline] > aio_write+0x63a/0x950 fs/aio.c:1600 > io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 > __do_sys_io_submit fs/aio.c:2078 [inline] > __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 > __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > It is because that skb->len requires at least sizeof(struct qrtr_ctrl_pkt) > in qrtr_tx_resume(). And skb->len equals to size in qrtr_endpoint_post(). > But size is less than sizeof(struct qrtr_ctrl_pkt) when qrtr_cb->type > equals to QRTR_TYPE_RESUME_TX in qrtr_endpoint_post() under the syzbot > scenario. This triggers the uninit variable access bug. > > Add size check when qrtr_cb->type equals to QRTR_TYPE_RESUME_TX in > qrtr_endpoint_post() to fix the bug. > > Fixes: 5fdeb0d372ab ("net: qrtr: Implement outgoing flow control") > Reported-by: syzbot+4436c9630a45820fda76@syzkaller.appspotmail.com > Link: https://syzkaller.appspot.com/bug?id=c14607f0963d27d5a3d5f4c8639b500909e43540 > Suggested-by: Manivannan Sadhasivam <mani@kernel.org> > Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> Reviewed-by: Simon Horman <simon.horman@corigine.com>
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Mon, 10 Apr 2023 09:23:52 +0800 you wrote: > Syzbot reported a bug as following: > > ===================================================== > BUG: KMSAN: uninit-value in qrtr_tx_resume+0x185/0x1f0 net/qrtr/af_qrtr.c:230 > qrtr_tx_resume+0x185/0x1f0 net/qrtr/af_qrtr.c:230 > qrtr_endpoint_post+0xf85/0x11b0 net/qrtr/af_qrtr.c:519 > qrtr_tun_write_iter+0x270/0x400 net/qrtr/tun.c:108 > call_write_iter include/linux/fs.h:2189 [inline] > aio_write+0x63a/0x950 fs/aio.c:1600 > io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 > __do_sys_io_submit fs/aio.c:2078 [inline] > __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 > __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 > do_syscall_x64 arch/x86/entry/common.c:50 [inline] > do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > [...] Here is the summary with links: - [net,v2] net: qrtr: Fix an uninit variable access bug in qrtr_tx_resume() https://git.kernel.org/netdev/net/c/6417070918de You are awesome, thank you!
diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c index 3a70255c8d02..76f0434d3d06 100644 --- a/net/qrtr/af_qrtr.c +++ b/net/qrtr/af_qrtr.c @@ -498,6 +498,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) if (!size || len != ALIGN(size, 4) + hdrlen) goto err; + if ((cb->type == QRTR_TYPE_NEW_SERVER || + cb->type == QRTR_TYPE_RESUME_TX) && + size < sizeof(struct qrtr_ctrl_pkt)) + goto err; + if (cb->dst_port != QRTR_PORT_CTRL && cb->type != QRTR_TYPE_DATA && cb->type != QRTR_TYPE_RESUME_TX) goto err; @@ -510,9 +515,6 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len) /* Remote node endpoint can bridge other distant nodes */ const struct qrtr_ctrl_pkt *pkt; - if (size < sizeof(*pkt)) - goto err; - pkt = data + hdrlen; qrtr_node_assign(node, le32_to_cpu(pkt->server.node)); }
Syzbot reported a bug as following: ===================================================== BUG: KMSAN: uninit-value in qrtr_tx_resume+0x185/0x1f0 net/qrtr/af_qrtr.c:230 qrtr_tx_resume+0x185/0x1f0 net/qrtr/af_qrtr.c:230 qrtr_endpoint_post+0xf85/0x11b0 net/qrtr/af_qrtr.c:519 qrtr_tun_write_iter+0x270/0x400 net/qrtr/tun.c:108 call_write_iter include/linux/fs.h:2189 [inline] aio_write+0x63a/0x950 fs/aio.c:1600 io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 __do_sys_io_submit fs/aio.c:2078 [inline] __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd Uninit was created at: slab_post_alloc_hook mm/slab.h:766 [inline] slab_alloc_node mm/slub.c:3452 [inline] __kmem_cache_alloc_node+0x71f/0xce0 mm/slub.c:3491 __do_kmalloc_node mm/slab_common.c:967 [inline] __kmalloc_node_track_caller+0x114/0x3b0 mm/slab_common.c:988 kmalloc_reserve net/core/skbuff.c:492 [inline] __alloc_skb+0x3af/0x8f0 net/core/skbuff.c:565 __netdev_alloc_skb+0x120/0x7d0 net/core/skbuff.c:630 qrtr_endpoint_post+0xbd/0x11b0 net/qrtr/af_qrtr.c:446 qrtr_tun_write_iter+0x270/0x400 net/qrtr/tun.c:108 call_write_iter include/linux/fs.h:2189 [inline] aio_write+0x63a/0x950 fs/aio.c:1600 io_submit_one+0x1d1c/0x3bf0 fs/aio.c:2019 __do_sys_io_submit fs/aio.c:2078 [inline] __se_sys_io_submit+0x293/0x770 fs/aio.c:2048 __x64_sys_io_submit+0x92/0xd0 fs/aio.c:2048 do_syscall_x64 arch/x86/entry/common.c:50 [inline] do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80 entry_SYSCALL_64_after_hwframe+0x63/0xcd It is because that skb->len requires at least sizeof(struct qrtr_ctrl_pkt) in qrtr_tx_resume(). And skb->len equals to size in qrtr_endpoint_post(). But size is less than sizeof(struct qrtr_ctrl_pkt) when qrtr_cb->type equals to QRTR_TYPE_RESUME_TX in qrtr_endpoint_post() under the syzbot scenario. This triggers the uninit variable access bug. Add size check when qrtr_cb->type equals to QRTR_TYPE_RESUME_TX in qrtr_endpoint_post() to fix the bug. Fixes: 5fdeb0d372ab ("net: qrtr: Implement outgoing flow control") Reported-by: syzbot+4436c9630a45820fda76@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?id=c14607f0963d27d5a3d5f4c8639b500909e43540 Suggested-by: Manivannan Sadhasivam <mani@kernel.org> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- v2: - Combine QRTR_TYPE_NEW_SERVER and QRTR_TYPE_RESUME_TX size check. --- net/qrtr/af_qrtr.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)