netdev - Re: [RFC] can: af_can: can_rcv() canfd_rcv(): Fix access of uninitialized memory or out of bounds

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: <b3ef9c62-2436-749a-4cf2-1faf55d8b542@hartkopp.net>
Date:   Mon, 24 Feb 2020 20:52:46 +0100
From:   Oliver Hartkopp <socketcan@...tkopp.net>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        "linux-can @ vger . kernel . org" <linux-can@...r.kernel.org>
Cc:     kernel@...gutronix.de, glider@...gle.com, kuba@...nel.org,
        netdev@...r.kernel.org,
        syzbot+9bcb0c9409066696d3aa@...kaller.appspotmail.com
Subject: Re: [RFC] can: af_can: can_rcv() canfd_rcv(): Fix access of
 uninitialized memory or out of bounds



On 24/02/2020 10.41, Marc Kleine-Budde wrote:
> Syzbot found the use of uninitialzied memory when injecting non conformant
> CANFD frames via a tun device into the kernel:
> 
> | BUG: KMSAN: uninit-value in number+0x9f8/0x2000 lib/vsprintf.c:459
> | CPU: 1 PID: 11897 Comm: syz-executor136 Not tainted 5.6.0-rc2-syzkaller #0
> | Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> | Call Trace:
> |  __dump_stack lib/dump_stack.c:77 [inline]
> |  dump_stack+0x1c9/0x220 lib/dump_stack.c:118
> |  kmsan_report+0xf7/0x1e0 mm/kmsan/kmsan_report.c:118
> |  __msan_warning+0x58/0xa0 mm/kmsan/kmsan_instr.c:215
> |  number+0x9f8/0x2000 lib/vsprintf.c:459
> |  vsnprintf+0x1d85/0x31b0 lib/vsprintf.c:2640
> |  vscnprintf+0xc2/0x180 lib/vsprintf.c:2677
> |  vprintk_store+0xef/0x11d0 kernel/printk/printk.c:1917
> |  vprintk_emit+0x2c0/0x860 kernel/printk/printk.c:1984
> |  vprintk_default+0x90/0xa0 kernel/printk/printk.c:2029
> |  vprintk_func+0x636/0x820 kernel/printk/printk_safe.c:386
> |  printk+0x18b/0x1d3 kernel/printk/printk.c:2062
> |  canfd_rcv+0x370/0x3a0 net/can/af_can.c:697
> |  __netif_receive_skb_one_core net/core/dev.c:5198 [inline]
> |  __netif_receive_skb net/core/dev.c:5312 [inline]
> |  netif_receive_skb_internal net/core/dev.c:5402 [inline]
> |  netif_receive_skb+0xe77/0xf20 net/core/dev.c:5461
> |  tun_rx_batched include/linux/skbuff.h:4321 [inline]
> |  tun_get_user+0x6aef/0x6f60 drivers/net/tun.c:1997
> |  tun_chr_write_iter+0x1f2/0x360 drivers/net/tun.c:2026
> |  call_write_iter include/linux/fs.h:1901 [inline]
> |  new_sync_write fs/read_write.c:483 [inline]
> |  __vfs_write+0xa5a/0xca0 fs/read_write.c:496
> |  vfs_write+0x44a/0x8f0 fs/read_write.c:558
> |  ksys_write+0x267/0x450 fs/read_write.c:611
> |  __do_sys_write fs/read_write.c:623 [inline]
> |  __se_sys_write+0x92/0xb0 fs/read_write.c:620
> |  __x64_sys_write+0x4a/0x70 fs/read_write.c:620
> |  do_syscall_64+0xb8/0x160 arch/x86/entry/common.c:296
> |  entry_SYSCALL_64_after_hwframe+0x44/0xa9
> 
> In canfd_rcv() the non conformant CANFD frame (i.e. skb too short) is detected,
> but the pr_warn_once() accesses uninitialized memory or the skb->data out of
> bounds to print the warning message.
> 
> This problem exists in both can_rcv() and canfd_rcv(). This patch removes the
> access to the skb->data from the pr_warn_once() in both functions.
> 
> Reported-by: syzbot+9bcb0c9409066696d3aa@...kaller.appspotmail.com
> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>

IMO the cfd->len is a relevant information for this warning message.

Won't it make sense, to check the skb->len inside pr_warn_once() whether 
the skb contains at least the cfd->len information before accessing it?

Btw. do you take this patch via your tree too?
https://marc.info/?l=linux-can&m=158039108004683

Regards,
Oliver

> ---
>   net/can/af_can.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/net/can/af_can.c b/net/can/af_can.c
> index 7511bfae2e0d..bd37922b0c9e 100644
> --- a/net/can/af_can.c
> +++ b/net/can/af_can.c
> @@ -677,8 +677,8 @@ static int can_rcv(struct sk_buff *skb, struct net_device *dev,
>   
>   	if (unlikely(dev->type != ARPHRD_CAN || skb->len != CAN_MTU ||
>   		     cfd->len > CAN_MAX_DLEN)) {
> -		pr_warn_once("PF_CAN: dropped non conform CAN skbuf: dev type %d, len %d, datalen %d\n",
> -			     dev->type, skb->len, cfd->len);
> +		pr_warn_once("PF_CAN: dropped non conform CAN skbuf: dev type %d, len %d\n",
> +			     dev->type, skb->len);
>   		kfree_skb(skb);
>   		return NET_RX_DROP;
>   	}
> @@ -694,8 +694,8 @@ static int canfd_rcv(struct sk_buff *skb, struct net_device *dev,
>   
>   	if (unlikely(dev->type != ARPHRD_CAN || skb->len != CANFD_MTU ||
>   		     cfd->len > CANFD_MAX_DLEN)) {
> -		pr_warn_once("PF_CAN: dropped non conform CAN FD skbuf: dev type %d, len %d, datalen %d\n",
> -			     dev->type, skb->len, cfd->len);
> +		pr_warn_once("PF_CAN: dropped non conform CAN FD skbuf: dev type %d, len %d\n",
> +			     dev->type, skb->len);
>   		kfree_skb(skb);
>   		return NET_RX_DROP;
>   	}
>