linux-kernel - Re: [PATCH bpf-next v3 1/3] bpf, sockmap: avoid using sk

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: <1e8c8e7a-23d9-4ed3-902a-8a4ba06f1f69@rbox.co>
Date: Thu, 20 Mar 2025 13:32:53 +0100
From: Michal Luczaj <mhal@...x.co>
To: Jiayuan Chen <jiayuan.chen@...ux.dev>, xiyou.wangcong@...il.com,
 john.fastabend@...il.com, jakub@...udflare.com
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, horms@...nel.org, andrii@...nel.org, eddyz87@...il.com,
 mykolal@...com, ast@...nel.org, daniel@...earbox.net, martin.lau@...ux.dev,
 song@...nel.org, yonghong.song@...ux.dev, kpsingh@...nel.org,
 sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, shuah@...nel.org,
 sgarzare@...hat.com, netdev@...r.kernel.org, bpf@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v3 1/3] bpf, sockmap: avoid using sk_socket after
 free when sending

On 3/17/25 10:22, Jiayuan Chen wrote:
> The sk->sk_socket is not locked or referenced, and during the call to
> skb_send_sock(), there is a race condition with the release of sk_socket.
> All types of sockets(tcp/udp/unix/vsock) will be affected.
> ...
> Some approach I tried
> ...
> 2. Increased the reference of sk_socket->file:
>    - If the user calls close(fd), we will do nothing because the reference
>      count is not set to 0. It's unexpected.

Have you considered bumping file's refcnt only for the time of
send/callback? Along the lines of:

static struct file *sock_get_file(struct sock *sk)
{
	struct file *file = NULL;
	struct socket *sock;

	rcu_read_lock();
	sock = sk->sk_socket;
	if (sock)
		file = get_file_active(&sock->file);
	rcu_read_unlock();

	return file;
}

static int sk_psock_handle_skb(struct sk_psock *psock, struct sk_buff *skb,
			       u32 off, u32 len, bool ingress)
{
	int err;

	if (!ingress) {
		struct sock *sk = psock->sk;
		struct file *file;
		...

		file = sock_get_file(sk);
		if (!file)
			return -EIO;

		err = skb_send_sock(sk, skb, off, len);
		fput(file);
		return err;
	}
	...
}

static void sk_psock_verdict_data_ready(struct sock *sk)
{
	struct file *file;
	...

	file = sock_get_file(sk);
	if (!file)
		return;

	copied = sk->sk_socket->ops->read_skb(sk, sk_psock_verdict_recv);
	fput(file);
	...
}