netdev - [PATCH bpf] xsk: fix xsk_diag use-after-free error during socket cleanup

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-next>] [day] [month] [year] [list]

Message-ID: <20230830151704.14855-1-magnus.karlsson@gmail.com>
Date: Wed, 30 Aug 2023 17:17:03 +0200
From: Magnus Karlsson <magnus.karlsson@...il.com>
To: magnus.karlsson@...el.com,
	bjorn@...nel.org,
	ast@...nel.org,
	daniel@...earbox.net,
	netdev@...r.kernel.org,
	maciej.fijalkowski@...el.com
Cc: jonathan.lemon@...il.com,
	bpf@...r.kernel.org,
	syzbot+822d1359297e2694f873@...kaller.appspotmail.com
Subject: [PATCH bpf] xsk: fix xsk_diag use-after-free error during socket cleanup

From: Magnus Karlsson <magnus.karlsson@...el.com>

Fix a use-after-free error that is possible if the xsk_diag interface
is used at the same time as the socket is being closed. In the early
days of AF_XDP, the way we tested that a socket was not bound or being
closed was to simply check if the netdevice pointer in the xsk socket
structure was NULL. Later, a better system was introduced by having an
explicit state variable in the xsk socket struct. For example, the
state of a socket that is going down is XSK_UNBOUND.

The commit in the Fixes tag below deleted the old way of signalling
that a socket is going down, setting dev to NULL. This in the belief
that all code using the old way had been exterminated. That was
unfortunately not true as the xsk diagnostics code was still using the
old way and thus does not work as intended when a socket is going
down. Fix this by introducing a test against the state variable. If
the socket is going down, simply abort the diagnostic's netlink
operation.

Fixes: 18b1ab7aa76b ("xsk: Fix race at socket teardown")
Reported-by: syzbot+822d1359297e2694f873@...kaller.appspotmail.com
Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
---
 net/xdp/xsk_diag.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/xdp/xsk_diag.c b/net/xdp/xsk_diag.c
index c014217f5fa7..da3100bfa1c5 100644
--- a/net/xdp/xsk_diag.c
+++ b/net/xdp/xsk_diag.c
@@ -111,6 +111,9 @@ static int xsk_diag_fill(struct sock *sk, struct sk_buff *nlskb,
 	sock_diag_save_cookie(sk, msg->xdiag_cookie);

 	mutex_lock(&xs->mutex);
+	if (xs->state == XSK_UNBOUND)
+		goto out_nlmsg_trim;
+
 	if ((req->xdiag_show & XDP_SHOW_INFO) && xsk_diag_put_info(xs, nlskb))
 		goto out_nlmsg_trim;

base-commit: 35d2b7ffffc1d9b3dc6c761010aa3338da49165b
-- 
2.42.0