Re: [PATCH bpf-next 2/6] bpf: Add ksock kfuncs

[Mahe Tardy <mahe.tardy@gmail.com>]

On Mon, Jul 06, 2026 at 11:01:27PM +0000, Kuniyuki Iwashima wrote:
> From: Mahe Tardy <mahe.tardy@gmail.com>
> Date: Mon,  6 Jul 2026 09:35:21 +0000
> > diff --git a/net/core/bpf_ksock.c b/net/core/bpf_ksock.c
> [...]
> > +static bool bpf_ksock_send_enter(struct bpf_ksock_send_guard *guard)
> 
> Can we do this better with assigning a single bit to task_struct ?

Yes, that would be way simpler but I avoided touching task_struct. If we
are okay with that then it could be simplified.

> 
> > +{
> > +	struct bpf_ksock_send_guard *entry;
> > +	struct hlist_bl_node *pos;
> > +	struct bpf_ksock_send_bucket *bucket;
> > +
> > +	bucket = bpf_ksock_send_bucket(current);
> > +	hlist_bl_lock(&bucket->head);
> > +	hlist_bl_for_each_entry(entry, pos, &bucket->head, node) {
> > +		if (entry->task == current) {
> > +			hlist_bl_unlock(&bucket->head);
> > +			return false;
> > +		}
> > +	}
> > +
> > +	guard->task = current;
> > +	INIT_HLIST_BL_NODE(&guard->node);
> > +	hlist_bl_add_head(&guard->node, &bucket->head);
> > +	hlist_bl_unlock(&bucket->head);
> > +	return true;
> > +}
> > +
> > +static void bpf_ksock_send_exit(struct bpf_ksock_send_guard *guard)
> > +{
> > +	struct bpf_ksock_send_bucket *bucket;
> > +
> > +	bucket = bpf_ksock_send_bucket(guard->task);
> > +	hlist_bl_lock(&bucket->head);
> > +	hlist_bl_del(&guard->node);
> > +	hlist_bl_unlock(&bucket->head);
> > +}
> [...]
> > +__bpf_kfunc struct bpf_ksock *
> > +bpf_ksock_create(const struct bpf_ksock_create_opts *opts, u32 opts__sz,
> > +		 int *err__uninit)
> > +{
> > +	struct bpf_ksock_create_opts opts_copy;
> > +	struct bpf_ksock *ks;
> > +	struct net *net;
> > +	int err;
> > +
> > +	/*
> > +	 * sock_create() derives the network namespace, credentials, and cgroup
> > +	 * from current. Kernel threads, including BPF workqueue callbacks, do
> > +	 * not carry the context of the task that invoked the BPF program.
> > +	 */
> > +	if (!bpf_ksock_has_user_task_context()) {
> > +		err = -EOPNOTSUPP;
> > +		goto err_out;
> > +	}
> > +
> > +	if (!opts || opts__sz != sizeof(struct bpf_ksock_create_opts)) {
> > +		err = -EINVAL;
> > +		goto err_out;
> > +	}
> > +
> > +	opts_copy = (struct bpf_ksock_create_opts){
> > +		.family = READ_ONCE(opts->family),
> > +		.type = READ_ONCE(opts->type),
> > +		.protocol = READ_ONCE(opts->protocol),
> > +		.reserved = READ_ONCE(opts->reserved),
> > +	};
> > +
> > +	if (opts_copy.reserved) {
> > +		err = -EINVAL;
> > +		goto err_out;
> > +	}
> > +
> > +	if (opts_copy.family != AF_INET && opts_copy.family != AF_INET6) {
> > +		err = -EAFNOSUPPORT;
> > +		goto err_out;
> > +	}
> > +
> > +	if (opts_copy.type != SOCK_DGRAM) {
> > +		err = -EPROTONOSUPPORT;
> > +		goto err_out;
> > +	}
> > +
> > +	if (opts_copy.protocol != IPPROTO_UDP && opts_copy.protocol != 0) {
> > +		err = -EPROTONOSUPPORT;
> > +		goto err_out;
> > +	}
> > +
> > +	ks = kzalloc_obj(*ks);
> > +	if (!ks) {
> > +		err = -ENOMEM;
> > +		goto err_out;
> > +	}
> > +
> > +	net = current->nsproxy->net_ns;
> > +	if (!bpf_ksock_net_try_charge(net)) {
> > +		err = -ENOSPC;
> > +		goto err_free;
> > +	}
> > +
> > +	/*
> > +	 * Use the normal current-task socket path so LSM/cgroup policy,
> > +	 * socket labels, and the active netns reference match a socket(2)
> > +	 * created by the BPF program's caller.
> > +	 */
> > +	err = sock_create(opts_copy.family, opts_copy.type, opts_copy.protocol,
> > +			  &ks->sock);
> 
> This also triggers LSM in __sock_create() so needs the loop
> detection, no ?
> 
> Same for __sys_bind_socket() and __sys_connect_socket().

This should be okay as create/bind/connect can only be called from
SYSCALL progs and not LSM progs. We only care about recursion from the
kfuncs in the ksock_kfunc_btf_ids group which are also allowed from LSM
progs (release/acquire/send).

> 
> 
> > +	if (err)
> > +		goto err_uncharge;
> > +
> > +	ks->sock->sk->sk_rcvbuf = SOCK_MIN_RCVBUF;
> > +	ks->sock->sk->sk_userlocks |= SOCK_RCVBUF_LOCK;
> > +
> > +	refcount_set(&ks->usage, 1);
> > +	put_unaligned(0, err__uninit);
> > +	return ks;
> > +
> > +err_uncharge:
> > +	bpf_ksock_net_uncharge(net);
> > +err_free:
> > +	kfree(ks);
> > +err_out:
> > +	put_unaligned(err, err__uninit);
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * bpf_ksock_bind() - Bind a BPF kernel socket to a local address.
> > + * @ks:		The BPF kernel socket context.
> > + * @opts:	Pointer to struct bpf_ksock_addr_opts with local address.
> > + * @opts__sz:	Size of the opts struct.
> > + *
> > + * Binds the socket to the specified local address and port.
> > + * This is optional; if not called, the kernel will auto-assign.
> > + *
> > + * This function may sleep while binding the socket, so it can only be used in
> > + * sleepable BPF programs (SYSCALL).
> > + *
> > + * Return: 0 on success, negative errno on error.
> > + */
> > +__bpf_kfunc int bpf_ksock_bind(struct bpf_ksock *ks,
> > +			       const struct bpf_ksock_addr_opts *opts,
> > +			       u32 opts__sz)
> > +{
> > +	struct sockaddr_storage addr = {};
> 
> nit: we don't need to initialise here since bpf_ksock_get_addr() does.

Ah actually I didn't know that compound literal in C would zero the rest
of the struct. Thanks, let's remove those two then.

> 
> > +	int addrlen;
> > +
> > +	if (!bpf_ksock_has_user_task_context())
> > +		return -EOPNOTSUPP;
> > +
> > +	addrlen = bpf_ksock_get_addr(opts, opts__sz, &addr);
> > +	if (addrlen < 0)
> > +		return addrlen;
> > +
> > +	return __sys_bind_socket(ks->sock, &addr, addrlen);
> > +}
> > +
> > +/**
> > + * bpf_ksock_connect() - Connect a BPF kernel socket to a remote address.
> > + * @ks:		The BPF kernel socket context.
> > + * @opts:	Pointer to struct bpf_ksock_addr_opts with remote address.
> > + * @opts__sz:	Size of the opts struct.
> > + *
> > + * Connects the socket to the specified remote address and port.
> > + *
> > + * This function may sleep while connecting the socket, so it can only be used
> > + * in sleepable BPF programs (SYSCALL).
> > + *
> > + * Return: 0 on success, negative errno on error.
> > + */
> > +__bpf_kfunc int bpf_ksock_connect(struct bpf_ksock *ks,
> > +				  const struct bpf_ksock_addr_opts *opts,
> > +				  u32 opts__sz)
> > +{
> > +	struct sockaddr_storage addr = {};
> 
> Same.

Noted