netdev - Re: [PATCH net 2/2] vsock/bpf: return early if transport is not assigned

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: <sho5ird455tiirzbsgjug6owi2leknai4xu45ddnesynb632oz@owq5b56n3f6h>
Date: Thu, 9 Jan 2025 10:24:30 +0100
From: Luigi Leonardi <leonardi@...hat.com>
To: Stefano Garzarella <sgarzare@...hat.com>
Cc: netdev@...r.kernel.org, Simon Horman <horms@...nel.org>, 
	Stefan Hajnoczi <stefanha@...hat.com>, linux-kernel@...r.kernel.org, Eric Dumazet <edumazet@...gle.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, Wongi Lee <qwerty@...ori.io>, 
	"David S. Miller" <davem@...emloft.net>, Paolo Abeni <pabeni@...hat.com>, 
	Jason Wang <jasowang@...hat.com>, Bobby Eshleman <bobby.eshleman@...edance.com>, 
	virtualization@...ts.linux.dev, Eugenio Pérez <eperezma@...hat.com>, 
	bpf@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>, 
	"Michael S. Tsirkin" <mst@...hat.com>, Hyunwoo Kim <v4bel@...ori.io>, Michal Luczaj <mhal@...x.co>, 
	kvm@...r.kernel.org
Subject: Re: [PATCH net 2/2] vsock/bpf: return early if transport is not
 assigned

On Wed, Jan 08, 2025 at 07:06:17PM +0100, Stefano Garzarella wrote:
>Some of the core functions can only be called if the transport
>has been assigned.
>
>As Michal reported, a socket might have the transport at NULL,
>for example after a failed connect(), causing the following trace:
>
>    BUG: kernel NULL pointer dereference, address: 00000000000000a0
>    #PF: supervisor read access in kernel mode
>    #PF: error_code(0x0000) - not-present page
>    PGD 12faf8067 P4D 12faf8067 PUD 113670067 PMD 0
>    Oops: Oops: 0000 [#1] PREEMPT SMP NOPTI
>    CPU: 15 UID: 0 PID: 1198 Comm: a.out Not tainted 6.13.0-rc2+
>    RIP: 0010:vsock_connectible_has_data+0x1f/0x40
>    Call Trace:
>     vsock_bpf_recvmsg+0xca/0x5e0
>     sock_recvmsg+0xb9/0xc0
>     __sys_recvfrom+0xb3/0x130
>     __x64_sys_recvfrom+0x20/0x30
>     do_syscall_64+0x93/0x180
>     entry_SYSCALL_64_after_hwframe+0x76/0x7e
>
>So we need to check the `vsk->transport` in vsock_bpf_recvmsg(),
>especially for connected sockets (stream/seqpacket) as we already
>do in __vsock_connectible_recvmsg().
>
>Fixes: 634f1a7110b4 ("vsock: support sockmap")
>Reported-by: Michal Luczaj <mhal@...x.co>
>Closes: https://lore.kernel.org/netdev/5ca20d4c-1017-49c2-9516-f6f75fd331e9@rbox.co/
>Signed-off-by: Stefano Garzarella <sgarzare@...hat.com>
>---
> net/vmw_vsock/vsock_bpf.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c
>index 4aa6e74ec295..f201d9eca1df 100644
>--- a/net/vmw_vsock/vsock_bpf.c
>+++ b/net/vmw_vsock/vsock_bpf.c
>@@ -77,6 +77,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
> 			     size_t len, int flags, int *addr_len)
> {
> 	struct sk_psock *psock;
>+	struct vsock_sock *vsk;
> 	int copied;
>
> 	psock = sk_psock_get(sk);
>@@ -84,6 +85,13 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
> 		return __vsock_recvmsg(sk, msg, len, flags);
>
> 	lock_sock(sk);
>+	vsk = vsock_sk(sk);
>+
>+	if (!vsk->transport) {
>+		copied = -ENODEV;
>+		goto out;
>+	}
>+
> 	if (vsock_has_data(sk, psock) && sk_psock_queue_empty(psock)) {
> 		release_sock(sk);
> 		sk_psock_put(sk, psock);
>@@ -108,6 +116,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg,
> 		copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
> 	}
>
>+out:
> 	release_sock(sk);
> 	sk_psock_put(sk, psock);
>
>-- 
>2.47.1
>
LGTM!

Reviewed-By: Luigi Leonardi <leonardi@...hat.com>