netdev - Re: [PATCH net] sctp: fix uninit-value in sctp_inq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADvbK_eQaqSJmNDGwz5A9tAmb0y2rZwZXxdC52B4hjjWRGZtUA@mail.gmail.com>
Date: Tue, 29 Aug 2023 16:27:48 -0400
From: Xin Long <lucien.xin@...il.com>
To: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
Cc: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	linux-sctp@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, 
	syzbot+70a42f45e76bede082be@...kaller.appspotmail.com
Subject: Re: [PATCH net] sctp: fix uninit-value in sctp_inq_pop()

On Tue, Aug 29, 2023 at 3:14 AM Nikita Zhandarovich
<n.zhandarovich@...tech.ru> wrote:
>
> Syzbot identified a case [1] of uninitialized memory usage in
> sctp_inq_pop(), specifically in 'ch->length'.
>
> Fix the issue by ensuring that 'ch->length' reflects the size of
> 'sctp_chunkhdr *ch' before accessing it.
>
> [1]
> BUG: KMSAN: uninit-value in sctp_inq_pop+0x1597/0x1910 net/sctp/inqueue.c:205
>  sctp_inq_pop+0x1597/0x1910 net/sctp/inqueue.c:205
>  sctp_assoc_bh_rcv+0x1a7/0xc50 net/sctp/associola.c:997
>  sctp_inq_push+0x23e/0x2b0 net/sctp/inqueue.c:80
>  sctp_backlog_rcv+0x394/0xd80 net/sctp/input.c:331
>  sk_backlog_rcv include/net/sock.h:1115 [inline]
>  __release_sock+0x207/0x570 net/core/sock.c:2911
>  release_sock+0x6b/0x1e0 net/core/sock.c:3478
>  sctp_wait_for_connect+0x486/0x810 net/sctp/socket.c:9325
>  sctp_sendmsg_to_asoc+0x1ea7/0x1ee0 net/sctp/socket.c:1884
>  ...
>
> Uninit was stored to memory at:
>  sctp_inq_pop+0x151a/0x1910 net/sctp/inqueue.c:201
>  sctp_assoc_bh_rcv+0x1a7/0xc50 net/sctp/associola.c:997
>  sctp_inq_push+0x23e/0x2b0 net/sctp/inqueue.c:80
>  sctp_backlog_rcv+0x394/0xd80 net/sctp/input.c:331
>  sk_backlog_rcv include/net/sock.h:1115 [inline]
>  __release_sock+0x207/0x570 net/core/sock.c:2911
>  release_sock+0x6b/0x1e0 net/core/sock.c:3478
>  sctp_wait_for_connect+0x486/0x810 net/sctp/socket.c:9325
>  sctp_sendmsg_to_asoc+0x1ea7/0x1ee0 net/sctp/socket.c:1884
>  ...
>
> Uninit was created at:
>  slab_post_alloc_hook+0x12d/0xb60 mm/slab.h:716
>  slab_alloc_node mm/slub.c:3451 [inline]
>  __kmem_cache_alloc_node+0x4ff/0x8b0 mm/slub.c:3490
>  __do_kmalloc_node mm/slab_common.c:965 [inline]
>  __kmalloc_node_track_caller+0x118/0x3c0 mm/slab_common.c:986
>  kmalloc_reserve+0x248/0x470 net/core/skbuff.c:585
>  __alloc_skb+0x318/0x740 net/core/skbuff.c:654
>  alloc_skb include/linux/skbuff.h:1288 [inline]
>  sctp_packet_pack net/sctp/output.c:472 [inline]
>  sctp_packet_transmit+0x1729/0x4150 net/sctp/output.c:621
>  sctp_outq_flush_transports net/sctp/outqueue.c:1173 [inline]
>  ...
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-and-tested-by: syzbot+70a42f45e76bede082be@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=70a42f45e76bede082be
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@...tech.ru>
> ---
>  net/sctp/inqueue.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/net/sctp/inqueue.c b/net/sctp/inqueue.c
> index 7182c5a450fb..98ce9524c87c 100644
> --- a/net/sctp/inqueue.c
> +++ b/net/sctp/inqueue.c
> @@ -197,6 +197,7 @@ struct sctp_chunk *sctp_inq_pop(struct sctp_inq *queue)
>                 }
>         }
>
> +       ch->length = htons(sizeof(*ch));
>         chunk->chunk_hdr = ch;
>         chunk->chunk_end = ((__u8 *)ch) + SCTP_PAD4(ntohs(ch->length));
>         skb_pull(chunk->skb, sizeof(*ch));
> --
> 2.25.1
>
Hi, Nikita

You can't just overwrite "ch->length", "ch" is the header of the received chunk.
if it says ch->length is Uninit, it means either the chunk parsing in
the receiver
is overflow or the format of the chunk created in the sender is incorrect.

If you can reproduce it stably, I suggest you start from sctp_inq_pop() and
print out the skb info and data in there, and see if it's a normal chunk.

Thanks.