netdev - Re: [PATCH net] net/smc: prevent UAF in inet

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <7250f49f-bfc0-4d7e-8de8-0468ff600a75@linux.alibaba.com>
Date: Wed, 24 Jul 2024 10:29:58 +0800
From: "D. Wythe" <alibuda@...ux.alibaba.com>
To: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com,
 syzbot <syzkaller@...glegroups.com>, Wenjia Zhang <wenjia@...ux.ibm.com>,
 Dust Li <dust.li@...ux.alibaba.com>, Niklas Schnelle <schnelle@...ux.ibm.com>
Subject: Re: [PATCH net] net/smc: prevent UAF in inet_create()



On 7/24/24 1:58 AM, Eric Dumazet wrote:
> Following syzbot repro crashes the kernel:
>
> socketpair(0x2, 0x1, 0x100, &(0x7f0000000140)) (fail_nth: 13)
>
> Fix this by not calling sk_common_release() from smc_create_clcsk().
>
> Stack trace:
> socket: no more sockets
> ------------[ cut here ]------------
> refcount_t: underflow; use-after-free.
>   WARNING: CPU: 1 PID: 5092 at lib/refcount.c:28 refcount_warn_saturate+0x15a/0x1d0 lib/refcount.c:28
> Modules linked in:
> CPU: 1 PID: 5092 Comm: syz-executor424 Not tainted 6.10.0-syzkaller-04483-g0be9ae5486cd #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/27/2024
>   RIP: 0010:refcount_warn_saturate+0x15a/0x1d0 lib/refcount.c:28
> Code: 80 f3 1f 8c e8 e7 69 a8 fc 90 0f 0b 90 90 eb 99 e8 cb 4f e6 fc c6 05 8a 8d e8 0a 01 90 48 c7 c7 e0 f3 1f 8c e8 c7 69 a8 fc 90 <0f> 0b 90 90 e9 76 ff ff ff e8 a8 4f e6 fc c6 05 64 8d e8 0a 01 90
> RSP: 0018:ffffc900034cfcf0 EFLAGS: 00010246
> RAX: 3b9fcde1c862f700 RBX: ffff888022918b80 RCX: ffff88807b39bc00
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: 0000000000000003 R08: ffffffff815878a2 R09: fffffbfff1c39d94
> R10: dffffc0000000000 R11: fffffbfff1c39d94 R12: 00000000ffffffe9
> R13: 1ffff11004523165 R14: ffff888022918b28 R15: ffff888022918b00
> FS:  00005555870e7380(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020000140 CR3: 000000007582e000 CR4: 00000000003506f0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>   <TASK>
>   inet_create+0xbaf/0xe70
>    __sock_create+0x490/0x920 net/socket.c:1571
>    sock_create net/socket.c:1622 [inline]
>    __sys_socketpair+0x2ca/0x720 net/socket.c:1769
>    __do_sys_socketpair net/socket.c:1822 [inline]
>    __se_sys_socketpair net/socket.c:1819 [inline]
>    __x64_sys_socketpair+0x9b/0xb0 net/socket.c:1819
>    do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>    do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
>   entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fbcb9259669
> Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 a1 1a 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007fffe931c6d8 EFLAGS: 00000246 ORIG_RAX: 0000000000000035
> RAX: ffffffffffffffda RBX: 00007fffe931c6f0 RCX: 00007fbcb9259669
> RDX: 0000000000000100 RSI: 0000000000000001 RDI: 0000000000000002
> RBP: 0000000000000002 R08: 00007fffe931c476 R09: 00000000000000a0
> R10: 0000000020000140 R11: 0000000000000246 R12: 00007fffe931c6ec
> R13: 431bde82d7b634db R14: 0000000000000001 R15: 0000000000000001
>   </TASK>

Oops, that's my bad, I forgot the differences in failure handling 
between smc_create and inet_create,
thanks for your fix.

>
> Fixes: d25a92ccae6b ("net/smc: Introduce IPPROTO_SMC")
> Reported-by: syzbot <syzkaller@...glegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: D. Wythe <alibuda@...ux.alibaba.com>
> Cc: Wenjia Zhang <wenjia@...ux.ibm.com>
> Cc: Dust Li <dust.li@...ux.alibaba.com>
> Cc: Niklas Schnelle <schnelle@...ux.ibm.com>
> ---
>   net/smc/af_smc.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index 73a875573e7ad5b7a95f7941e33f0d784a91d16d..31b5d8c8c34085b73b011c913cfe032f025cd2e0 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -3319,10 +3319,8 @@ int smc_create_clcsk(struct net *net, struct sock *sk, int family)
>   
>   	rc = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP,
>   			      &smc->clcsock);
> -	if (rc) {
> -		sk_common_release(sk);
> +	if (rc)
>   		return rc;
> -	}

Since __smc_create (for AF_SMC) does not call sk_common_release on failure,
I think it should be moved to __smc_create instead of beingdeleted.

Best wishes,
D. Wythe

>   
>   	/* smc_clcsock_release() does not wait smc->clcsock->sk's
>   	 * destruction;  its sk_state might not be TCP_CLOSE after