| 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
| ||
|
Message-ID: <20240118215854.40268-1-kuniyu@amazon.com>
Date: Thu, 18 Jan 2024 13:58:54 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <edumazet@...gle.com>
CC: <davem@...emloft.net>, <eric.dumazet@...il.com>, <kuba@...nel.org>,
<netdev@...r.kernel.org>, <pabeni@...hat.com>,
<syzbot+2a7024e9502df538e8ef@...kaller.appspotmail.com>, <kuniyu@...zon.com>
Subject: Re: [PATCH net] llc: make llc_ui_sendmsg() more robust against bonding changes
From: Eric Dumazet <edumazet@...gle.com>
Date: Thu, 18 Jan 2024 18:36:25 +0000
> syzbot was able to trick llc_ui_sendmsg(), allocating an skb with no
> headroom, but subsequently trying to push 14 bytes of Ethernet header [1]
>
> Like some others, llc_ui_sendmsg() releases the socket lock before
> calling sock_alloc_send_skb().
> Then it acquires it again, but does not redo all the sanity checks
> that were performed.
>
> This fix:
>
> - Uses LL_RESERVED_SPACE() to reserve space.
> - Check all conditions again after socket lock is held again.
> - Do not account Ethernet header for mtu limitation.
>
> [1]
>
> skbuff: skb_under_panic: text:ffff800088baa334 len:1514 put:14 head:ffff0000c9c37000 data:ffff0000c9c36ff2 tail:0x5dc end:0x6c0 dev:bond0
>
> kernel BUG at net/core/skbuff.c:193 !
> Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
> Modules linked in:
> CPU: 0 PID: 6875 Comm: syz-executor.0 Not tainted 6.7.0-rc8-syzkaller-00101-g0802e17d9aca-dirty #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023
> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> pc : skb_panic net/core/skbuff.c:189 [inline]
> pc : skb_under_panic+0x13c/0x140 net/core/skbuff.c:203
> lr : skb_panic net/core/skbuff.c:189 [inline]
> lr : skb_under_panic+0x13c/0x140 net/core/skbuff.c:203
> sp : ffff800096f97000
> x29: ffff800096f97010 x28: ffff80008cc8d668 x27: dfff800000000000
> x26: ffff0000cb970c90 x25: 00000000000005dc x24: ffff0000c9c36ff2
> x23: ffff0000c9c37000 x22: 00000000000005ea x21: 00000000000006c0
> x20: 000000000000000e x19: ffff800088baa334 x18: 1fffe000368261ce
> x17: ffff80008e4ed000 x16: ffff80008a8310f8 x15: 0000000000000001
> x14: 1ffff00012df2d58 x13: 0000000000000000 x12: 0000000000000000
> x11: 0000000000000001 x10: 0000000000ff0100 x9 : e28a51f1087e8400
> x8 : e28a51f1087e8400 x7 : ffff80008028f8d0 x6 : 0000000000000000
> x5 : 0000000000000001 x4 : 0000000000000001 x3 : ffff800082b78714
> x2 : 0000000000000001 x1 : 0000000100000000 x0 : 0000000000000089
> Call trace:
> skb_panic net/core/skbuff.c:189 [inline]
> skb_under_panic+0x13c/0x140 net/core/skbuff.c:203
> skb_push+0xf0/0x108 net/core/skbuff.c:2451
> eth_header+0x44/0x1f8 net/ethernet/eth.c:83
> dev_hard_header include/linux/netdevice.h:3188 [inline]
> llc_mac_hdr_init+0x110/0x17c net/llc/llc_output.c:33
> llc_sap_action_send_xid_c+0x170/0x344 net/llc/llc_s_ac.c:85
> llc_exec_sap_trans_actions net/llc/llc_sap.c:153 [inline]
> llc_sap_next_state net/llc/llc_sap.c:182 [inline]
> llc_sap_state_process+0x1ec/0x774 net/llc/llc_sap.c:209
> llc_build_and_send_xid_pkt+0x12c/0x1c0 net/llc/llc_sap.c:270
> llc_ui_sendmsg+0x7bc/0xb1c net/llc/af_llc.c:997
> sock_sendmsg_nosec net/socket.c:730 [inline]
> __sock_sendmsg net/socket.c:745 [inline]
> sock_sendmsg+0x194/0x274 net/socket.c:767
> splice_to_socket+0x7cc/0xd58 fs/splice.c:881
> do_splice_from fs/splice.c:933 [inline]
> direct_splice_actor+0xe4/0x1c0 fs/splice.c:1142
> splice_direct_to_actor+0x2a0/0x7e4 fs/splice.c:1088
> do_splice_direct+0x20c/0x348 fs/splice.c:1194
> do_sendfile+0x4bc/0xc70 fs/read_write.c:1254
> __do_sys_sendfile64 fs/read_write.c:1322 [inline]
> __se_sys_sendfile64 fs/read_write.c:1308 [inline]
> __arm64_sys_sendfile64+0x160/0x3b4 fs/read_write.c:1308
> __invoke_syscall arch/arm64/kernel/syscall.c:37 [inline]
> invoke_syscall+0x98/0x2b8 arch/arm64/kernel/syscall.c:51
> el0_svc_common+0x130/0x23c arch/arm64/kernel/syscall.c:136
> do_el0_svc+0x48/0x58 arch/arm64/kernel/syscall.c:155
> el0_svc+0x54/0x158 arch/arm64/kernel/entry-common.c:678
> el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:696
> el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:595
> Code: aa1803e6 aa1903e7 a90023f5 94792f6a (d4210000)
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-and-tested-by: syzbot+2a7024e9502df538e8ef@...kaller.appspotmail.com
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> ---
> net/llc/af_llc.c | 24 ++++++++++++++++--------
> 1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
> index 9b06c380866b53bcb395bf255587279db025d11d..20551cfb7da6d8dd098c906477895e26c080fe32 100644
> --- a/net/llc/af_llc.c
> +++ b/net/llc/af_llc.c
> @@ -928,14 +928,15 @@ static int llc_ui_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
> */
> static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> {
> + DECLARE_SOCKADDR(struct sockaddr_llc *, addr, msg->msg_name);
> struct sock *sk = sock->sk;
> struct llc_sock *llc = llc_sk(sk);
> - DECLARE_SOCKADDR(struct sockaddr_llc *, addr, msg->msg_name);
> int flags = msg->msg_flags;
> int noblock = flags & MSG_DONTWAIT;
> + int rc = -EINVAL, copied = 0, hdrlen, hh_len;
> struct sk_buff *skb = NULL;
> + struct net_device *dev;
> size_t size = 0;
> - int rc = -EINVAL, copied = 0, hdrlen;
>
> dprintk("%s: sending from %02X to %02X\n", __func__,
> llc->laddr.lsap, llc->daddr.lsap);
> @@ -955,22 +956,29 @@ static int llc_ui_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> if (rc)
> goto out;
> }
> - hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr);
> + dev = llc->dev;
> + hh_len = LL_RESERVED_SPACE(dev);
> + hdrlen = llc_ui_header_len(sk, addr);
> size = hdrlen + len;
> - if (size > llc->dev->mtu)
> - size = llc->dev->mtu;
> + size = min_t(size_t, size, READ_ONCE(dev->mtu));
> copied = size - hdrlen;
> rc = -EINVAL;
> if (copied < 0)
> goto out;
> release_sock(sk);
> - skb = sock_alloc_send_skb(sk, size, noblock, &rc);
> + skb = sock_alloc_send_skb(sk, hh_len + size, noblock, &rc);
> lock_sock(sk);
> if (!skb)
> goto out;
> - skb->dev = llc->dev;
> + if (sock_flag(sk, SOCK_ZAPPED) ||
Probably we need not check SOCK_ZAPPED again after llc_ui_autobind() ?
> + llc->dev != dev ||
> + hdrlen != llc_ui_header_len(sk, addr) ||
> + hh_len != LL_RESERVED_SPACE(dev) ||
> + size > READ_ONCE(dev->mtu))
> + goto out;
> + skb->dev = dev;
> skb->protocol = llc_proto_type(addr->sllc_arphrd);
> - skb_reserve(skb, hdrlen);
> + skb_reserve(skb, hh_len + hdrlen);
> rc = memcpy_from_msg(skb_put(skb, copied), msg, copied);
> if (rc)
> goto out;
> --
> 2.43.0.429.g432eaa2c6b-goog
Powered by blists - more mailing lists