Re: [PATCH net] net: qrtr: Fix an uninit variable access bug in qrtr_tx_resume()

[Paolo Abeni <pabeni@redhat.com>]

Hi,

On Tue, 2023-04-04 at 08:38 +0800, Ziyang Xuan (William) wrote:
> > On Mon, Apr 03, 2023 at 03:54:17PM +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.
> > > 
> > 
> > I'm not familiar with syzkaller. Can you please share the data that
> > was fuzzed
> > by the bot?
> > 
> > - Mani
> > 
> > > 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
> 
> Hello Manivannan Sadhasivam
> 
> See the above link, it's syzkaller dashboard link, you can find a C
> reproducer that will help you.

Hi Mani,

Are you satisfied with the information above? The patch LGTM and the
syzkaller report looks quite clear.

Thanks!

Paolo