On Mon, Feb 17, 2025 at 08:45:06PM +0100, Michal Luczaj wrote:
On 2/17/25 11:59, Stefano Garzarella wrote:
On Thu, Feb 13, 2025 at 12:58:50PM +0100, Michal Luczaj wrote:
In the spirit of commit 91751e248256 ("vsock: prevent null-ptr-deref in vsock_*[has_data|has_space]"), armorize the "impossible" cases with a warning.
Fixes: 634f1a7110b4 ("vsock: support sockmap") Signed-off-by: Michal Luczaj mhal@rbox.co
net/vmw_vsock/af_vsock.c | 3 +++ net/vmw_vsock/vsock_bpf.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 53a081d49d28ac1c04e7f8057c8a55e7b73cc131..7e3db87ae4333cf63327ec105ca99253569bb9fe 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1189,6 +1189,9 @@ static int vsock_read_skb(struct sock *sk, skb_read_actor_t read_actor) { struct vsock_sock *vsk = vsock_sk(sk);
- if (WARN_ON_ONCE(!vsk->transport))
return -ENODEV;
- return vsk->transport->read_skb(vsk, read_actor);
}
diff --git a/net/vmw_vsock/vsock_bpf.c b/net/vmw_vsock/vsock_bpf.c index f201d9eca1df2f8143638cf7a4d08671e8368c11..07b96d56f3a577af71021b1b8132743554996c4f 100644 --- a/net/vmw_vsock/vsock_bpf.c +++ b/net/vmw_vsock/vsock_bpf.c @@ -87,7 +87,7 @@ static int vsock_bpf_recvmsg(struct sock *sk, struct msghdr *msg, lock_sock(sk); vsk = vsock_sk(sk);
- if (!vsk->transport) {
- if (WARN_ON_ONCE(!vsk->transport)) { copied = -ENODEV; goto out; }
I'm not a sockmap expert, so I don't understand why here print an error.
Since there was already a check, I expected it to be a case that can happen, but instead calling `rcvmsg()` on a socket not yet connected is impossible?
That's right, calling vsock_bpf_recvmsg() on a not-yet-connected connectible socket is impossible since PATCH 1/4 of this series.
Okay, I get it now.
If you need to send a v2, can you write it in the commit description that the behavior changed in the previous patch and so in vsock_bpf_recvmsg() now we no longer expect to be called with a null transport.
In any case, LGTM:
Reviewed-by: Stefano Garzarella sgarzare@redhat.com
That is because to reach vsock_bpf_recvmsg(), you must have sock's proto replaced in vsock_bpf_update_proto(). For that you need to run sock_map_init_proto(), which you can't because the patched sock_map_sk_state_allowed() will stop you.