Skip to content

Commit

Permalink
Merge branch 'bpf-vsock-fix-poll-and-close'
Browse files Browse the repository at this point in the history
Michal Luczaj says:

====================
bpf, vsock: Fix poll() and close()

Two small fixes for vsock: poll() missing a queue check, and close() not
invoking sockmap cleanup.

Signed-off-by: Michal Luczaj <mhal@rbox.co>
Acked-by: John Fastabend <john.fastabend@gmail.com>
---
====================

Link: https://patch.msgid.link/20241118-vsock-bpf-poll-close-v1-0-f1b9669cacdc@rbox.co
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
Alexei Starovoitov committed Nov 25, 2024
2 parents 8618f5f + 5157454 commit 20a39ea
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 27 deletions.
70 changes: 43 additions & 27 deletions net/vmw_vsock/af_vsock.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,14 @@
static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr);
static void vsock_sk_destruct(struct sock *sk);
static int vsock_queue_rcv_skb(struct sock *sk, struct sk_buff *skb);
static void vsock_close(struct sock *sk, long timeout);

/* Protocol family. */
struct proto vsock_proto = {
.name = "AF_VSOCK",
.owner = THIS_MODULE,
.obj_size = sizeof(struct vsock_sock),
.close = vsock_close,
#ifdef CONFIG_BPF_SYSCALL
.psock_update_sk_prot = vsock_bpf_update_proto,
#endif
Expand Down Expand Up @@ -797,39 +799,37 @@ static bool sock_type_connectible(u16 type)

static void __vsock_release(struct sock *sk, int level)
{
if (sk) {
struct sock *pending;
struct vsock_sock *vsk;

vsk = vsock_sk(sk);
pending = NULL; /* Compiler warning. */
struct vsock_sock *vsk;
struct sock *pending;

/* When "level" is SINGLE_DEPTH_NESTING, use the nested
* version to avoid the warning "possible recursive locking
* detected". When "level" is 0, lock_sock_nested(sk, level)
* is the same as lock_sock(sk).
*/
lock_sock_nested(sk, level);
vsk = vsock_sk(sk);
pending = NULL; /* Compiler warning. */

if (vsk->transport)
vsk->transport->release(vsk);
else if (sock_type_connectible(sk->sk_type))
vsock_remove_sock(vsk);
/* When "level" is SINGLE_DEPTH_NESTING, use the nested
* version to avoid the warning "possible recursive locking
* detected". When "level" is 0, lock_sock_nested(sk, level)
* is the same as lock_sock(sk).
*/
lock_sock_nested(sk, level);

sock_orphan(sk);
sk->sk_shutdown = SHUTDOWN_MASK;
if (vsk->transport)
vsk->transport->release(vsk);
else if (sock_type_connectible(sk->sk_type))
vsock_remove_sock(vsk);

skb_queue_purge(&sk->sk_receive_queue);
sock_orphan(sk);
sk->sk_shutdown = SHUTDOWN_MASK;

/* Clean up any sockets that never were accepted. */
while ((pending = vsock_dequeue_accept(sk)) != NULL) {
__vsock_release(pending, SINGLE_DEPTH_NESTING);
sock_put(pending);
}
skb_queue_purge(&sk->sk_receive_queue);

release_sock(sk);
sock_put(sk);
/* Clean up any sockets that never were accepted. */
while ((pending = vsock_dequeue_accept(sk)) != NULL) {
__vsock_release(pending, SINGLE_DEPTH_NESTING);
sock_put(pending);
}

release_sock(sk);
sock_put(sk);
}

static void vsock_sk_destruct(struct sock *sk)
Expand Down Expand Up @@ -901,9 +901,22 @@ void vsock_data_ready(struct sock *sk)
}
EXPORT_SYMBOL_GPL(vsock_data_ready);

/* Dummy callback required by sockmap.
* See unconditional call of saved_close() in sock_map_close().
*/
static void vsock_close(struct sock *sk, long timeout)
{
}

static int vsock_release(struct socket *sock)
{
__vsock_release(sock->sk, 0);
struct sock *sk = sock->sk;

if (!sk)
return 0;

sk->sk_prot->close(sk, 0);
__vsock_release(sk, 0);
sock->sk = NULL;
sock->state = SS_FREE;

Expand Down Expand Up @@ -1054,6 +1067,9 @@ static __poll_t vsock_poll(struct file *file, struct socket *sock,
mask |= EPOLLRDHUP;
}

if (sk_is_readable(sk))
mask |= EPOLLIN | EPOLLRDNORM;

if (sock->type == SOCK_DGRAM) {
/* For datagram sockets we can read if there is something in
* the queue and write as long as the socket isn't shutdown for
Expand Down
77 changes: 77 additions & 0 deletions tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,35 @@ static void test_sockmap_create_update_free(enum bpf_map_type map_type)
close(s);
}

static void test_sockmap_vsock_delete_on_close(void)
{
int err, c, p, map;
const int zero = 0;

err = create_pair(AF_VSOCK, SOCK_STREAM, &c, &p);
if (!ASSERT_OK(err, "create_pair(AF_VSOCK)"))
return;

map = bpf_map_create(BPF_MAP_TYPE_SOCKMAP, NULL, sizeof(int),
sizeof(int), 1, NULL);
if (!ASSERT_GE(map, 0, "bpf_map_create")) {
close(c);
goto out;
}

err = bpf_map_update_elem(map, &zero, &c, BPF_NOEXIST);
close(c);
if (!ASSERT_OK(err, "bpf_map_update"))
goto out;

err = bpf_map_update_elem(map, &zero, &p, BPF_NOEXIST);
ASSERT_OK(err, "after close(), bpf_map_update");

out:
close(p);
close(map);
}

static void test_skmsg_helpers(enum bpf_map_type map_type)
{
struct test_skmsg_load_helpers *skel;
Expand Down Expand Up @@ -937,12 +966,58 @@ static void test_sockmap_same_sock(void)
test_sockmap_pass_prog__destroy(skel);
}

static void test_sockmap_skb_verdict_vsock_poll(void)
{
struct test_sockmap_pass_prog *skel;
int err, map, conn, peer;
struct bpf_program *prog;
struct bpf_link *link;
char buf = 'x';
int zero = 0;

skel = test_sockmap_pass_prog__open_and_load();
if (!ASSERT_OK_PTR(skel, "open_and_load"))
return;

if (create_pair(AF_VSOCK, SOCK_STREAM, &conn, &peer))
goto destroy;

prog = skel->progs.prog_skb_verdict;
map = bpf_map__fd(skel->maps.sock_map_rx);
link = bpf_program__attach_sockmap(prog, map);
if (!ASSERT_OK_PTR(link, "bpf_program__attach_sockmap"))
goto close;

err = bpf_map_update_elem(map, &zero, &conn, BPF_ANY);
if (!ASSERT_OK(err, "bpf_map_update_elem"))
goto detach;

if (xsend(peer, &buf, 1, 0) != 1)
goto detach;

err = poll_read(conn, IO_TIMEOUT_SEC);
if (!ASSERT_OK(err, "poll"))
goto detach;

if (xrecv_nonblock(conn, &buf, 1, 0) != 1)
FAIL("xrecv_nonblock");
detach:
bpf_link__detach(link);
close:
xclose(conn);
xclose(peer);
destroy:
test_sockmap_pass_prog__destroy(skel);
}

void test_sockmap_basic(void)
{
if (test__start_subtest("sockmap create_update_free"))
test_sockmap_create_update_free(BPF_MAP_TYPE_SOCKMAP);
if (test__start_subtest("sockhash create_update_free"))
test_sockmap_create_update_free(BPF_MAP_TYPE_SOCKHASH);
if (test__start_subtest("sockmap vsock delete on close"))
test_sockmap_vsock_delete_on_close();
if (test__start_subtest("sockmap sk_msg load helpers"))
test_skmsg_helpers(BPF_MAP_TYPE_SOCKMAP);
if (test__start_subtest("sockhash sk_msg load helpers"))
Expand Down Expand Up @@ -997,4 +1072,6 @@ void test_sockmap_basic(void)
test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKMAP);
if (test__start_subtest("sockhash sk_msg attach sockhash helpers with link"))
test_skmsg_helpers_with_link(BPF_MAP_TYPE_SOCKHASH);
if (test__start_subtest("sockmap skb_verdict vsock poll"))
test_sockmap_skb_verdict_vsock_poll();
}

0 comments on commit 20a39ea

Please sign in to comment.