| From 0a140d63a92b47108c5cdfe05d93bdf13cdeec98 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 10 Apr 2023 09:23:52 +0800 |
| Subject: net: qrtr: Fix an uninit variable access bug in qrtr_tx_resume() |
| |
| From: Ziyang Xuan <william.xuanziyang@huawei.com> |
| |
| [ Upstream commit 6417070918de3bcdbe0646e7256dae58fd8083ba ] |
| |
| 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> |
| Link: https://lore.kernel.org/r/20230410012352.3997823-1-william.xuanziyang@huawei.com |
| Signed-off-by: Paolo Abeni <pabeni@redhat.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/qrtr/af_qrtr.c | 8 +++++--- |
| 1 file changed, 5 insertions(+), 3 deletions(-) |
| |
| diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c |
| index 3a70255c8d02f..76f0434d3d06a 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)); |
| } |
| -- |
| 2.39.2 |
| |