Message-ID: <CAHC9VhTM14E7Mz_ToVEqpW0CQr0KEfpwZOnSzTSYdMxX55k4yQ@mail.gmail.com>
Date: Thu, 22 May 2025 18:30:55 -0400
From: Paul Moore <paul@...l-moore.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, 
	Huw Davies <huw@...eweavers.com>, Kuniyuki Iwashima <kuni1840@...il.com>, netdev@...r.kernel.org, 
	linux-security-module@...r.kernel.org, syzkaller <syzkaller@...glegroups.com>, 
	John Cheung <john.cs.hey@...il.com>
Subject: Re: [PATCH v1 net] calipso: Don't call calipso functions for AF_INET sk.

On Thu, May 22, 2025 at 6:19 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>
> syzkaller reported a null-ptr-deref in txopt_get(). [0]
>
> The offset 0x70 was of struct ipv6_txoptions in struct ipv6_pinfo,
> so struct ipv6_pinfo was NULL there.
>
> However, this never happens for IPv6 sockets as inet_sk(sk)->pinet6
> is always set in inet6_create(), meaning the socket was not IPv6 one.
>
> The root cause is missing validation in netlbl_conn_setattr().
>
> netlbl_conn_setattr() switches branches based on struct
> sockaddr.sa_family, which is passed from userspace.  However,
> netlbl_conn_setattr() does not check if the address family matches
> the socket.
>
> The syzkaller must have called connect() for an IPv6 address on
> an IPv4 socket.
>
> We have a proper validation in tcp_v[46]_connect(), but
> security_socket_connect() is called in the earlier stage.
>
> Let's copy the validation to netlbl_conn_setattr().
>
> [0]:
> Oops: general protection fault, probably for non-canonical address 0xdffffc000000000e: 0000 [#1] PREEMPT SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000070-0x0000000000000077]
> CPU: 2 UID: 0 PID: 12928 Comm: syz.9.1677 Not tainted 6.12.0 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> RIP: 0010:txopt_get include/net/ipv6.h:390 [inline]
> RIP: 0010:
> Code: 02 00 00 49 8b ac 24 f8 02 00 00 e8 84 69 2a fd e8 ff 00 16 fd 48 8d 7d 70 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 53 02 00 00 48 8b 6d 70 48 85 ed 0f 84 ab 01 00
> RSP: 0018:ffff88811b8afc48 EFLAGS: 00010212
> RAX: dffffc0000000000 RBX: 1ffff11023715f8a RCX: ffffffff841ab00c
> RDX: 000000000000000e RSI: ffffc90007d9e000 RDI: 0000000000000070
> RBP: 0000000000000000 R08: ffffed1023715f9d R09: ffffed1023715f9e
> R10: ffffed1023715f9d R11: 0000000000000003 R12: ffff888123075f00
> R13: ffff88810245bd80 R14: ffff888113646780 R15: ffff888100578a80
> FS:  00007f9019bd7640(0000) GS:ffff8882d2d00000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f901b927bac CR3: 0000000104788003 CR4: 0000000000770ef0
> PKRU: 80000000
> Call Trace:
>  <TASK>
>  calipso_sock_setattr+0x56/0x80 net/netlabel/netlabel_calipso.c:557
>  netlbl_conn_setattr+0x10c/0x280 net/netlabel/netlabel_kapi.c:1177
>  selinux_netlbl_socket_connect_helper+0xd3/0x1b0 security/selinux/netlabel.c:569
>  selinux_netlbl_socket_connect_locked security/selinux/netlabel.c:597 [inline]
>  selinux_netlbl_socket_connect+0xb6/0x100 security/selinux/netlabel.c:615
>  selinux_socket_connect+0x5f/0x80 security/selinux/hooks.c:4931
>  security_socket_connect+0x50/0xa0 security/security.c:4598
>  __sys_connect_file+0xa4/0x190 net/socket.c:2067
>  __sys_connect+0x12c/0x170 net/socket.c:2088
>  __do_sys_connect net/socket.c:2098 [inline]
>  __se_sys_connect net/socket.c:2095 [inline]
>  __x64_sys_connect+0x73/0xb0 net/socket.c:2095
>  do_syscall_x64 arch/x86/entry/common.c:52 [inline]
>  do_syscall_64+0xaa/0x1b0 arch/x86/entry/common.c:83
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f901b61a12d
> Code: 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 f3 0f 1e fa 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 a8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007f9019bd6fa8 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
> RAX: ffffffffffffffda RBX: 00007f901b925fa0 RCX: 00007f901b61a12d
> RDX: 000000000000001c RSI: 0000200000000140 RDI: 0000000000000003
> RBP: 00007f901b701505 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: 0000000000000000 R14: 00007f901b5b62a0 R15: 00007f9019bb7000
>  </TASK>
> Modules linked in:
>
> Fixes: ceba1832b1b2 ("calipso: Set the calipso socket label to match the secattr.")
> Reported-by: syzkaller <syzkaller@...glegroups.com>
> Reported-by: John Cheung <john.cs.hey@...il.com>
> Closes: https://lore.kernel.org/netdev/CAP=Rh=M1LzunrcQB1fSGauMrJrhL6GGps5cPAKzHJXj6GQV+-g@mail.gmail.com/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> ---
>  net/netlabel/netlabel_kapi.c | 3 +++
>  1 file changed, 3 insertions(+)

Looks good to me, thanks for tracking this down and fixing it :)

Acked-by: Paul Moore <paul@...l-moore.com>

> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index cd9160bbc919..6ea16138582c 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -1165,6 +1165,9 @@ int netlbl_conn_setattr(struct sock *sk,
>                 break;
>  #if IS_ENABLED(CONFIG_IPV6)
>         case AF_INET6:
> +               if (sk->sk_family != AF_INET6)
> +                       return -EAFNOSUPPORT;
> +
>                 addr6 = (struct sockaddr_in6 *)addr;
>                 entry = netlbl_domhsh_getentry_af6(secattr->domain,
>                                                    &addr6->sin6_addr);
> --
> 2.49.0

-- 
paul-moore.com

Powered by blists - more mailing lists