Re: [PATCH bpf-next v2] bpf: allow ksock connect/send calls from wq

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

On Tue, Oct 13, 2026 at 06:35:59PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] selftests/bpf: u64 map key causes workqueue double-init on big-endian
> --
> 
> commit 5eb6c40a460d2cca2d0dee2b208a0164f311d369
> Author: Mahe Tardy <mahe.tardy@gmail.com>
> 
> bpf: allow ksock connect/send calls from wq
> 
> This commit relaxes the constraint preventing the use of ksock kfuncs from
> asynchronous workqueues. It also updates the ksock_wq selftest to verify
> that connect() and send() function properly when called from workqueues.
> 
> > diff --git a/tools/testing/selftests/bpf/progs/ksock_wq.c b/tools/testing/selftests/bpf/progs/ksock_wq.c
> > index 16a1873d132ed..290736e219e1d 100644
> > --- a/tools/testing/selftests/bpf/progs/ksock_wq.c
> > +++ b/tools/testing/selftests/bpf/progs/ksock_wq.c
> [ ... ]
> > @@ -43,20 +73,40 @@ static int ksock_wq_callback(void *map, int *key, void *value)
> >  SEC("syscall")
> >  int ksock_wq_start(void *ctx)
> >  {
> > +	struct bpf_ksock_create_opts opts = {
> > +		.family = AF_INET,
> > +		.type = SOCK_DGRAM,
> > +		.protocol = IPPROTO_UDP,
> > +	};
> >  	struct ksock_wq_value *value;
> > -	u32 key = 0;
> > -	int err;
> > +	struct bpf_ksock *ks;
> > +	u64 key = 0;
> > +	int err = 0;
> >  
> > -	value = bpf_map_lookup_elem(&work_map, &key);
> > -	if (!value)
> > -		return -ENOENT;
> > -	err = bpf_wq_init(&value->work, &work_map, 0);
> > -	if (err)
> > +	ks = bpf_ksock_create(&opts, sizeof(opts), &err);
> > +	if (!ks)
> >  		return err;
> > -	err = bpf_wq_set_callback(&value->work, ksock_wq_callback, 0);
> > +	err = ksock_ctx_insert(ks);
> >  	if (err)
> >  		return err;
> > -	return bpf_wq_start(&value->work, 0);
> > +	for (key = 0; key < 2; key++) {
> > +		value = bpf_map_lookup_elem(&work_map, &key);
> 
> [Severity: Medium]
> Can this cause a workqueue double-init on big-endian architectures?
> 
> The work_map map is defined with a u32 key, but the key variable here was
> changed to u64. On big-endian systems, passing &key to bpf_map_lookup_elem()
> will read the most significant 32 bits, which will evaluate to 0 for both
> key = 0 and key = 1.
> 
> Because of this, the loop would fetch the map element for index 0 twice.
> During the second iteration, bpf_wq_init() would be called on value->work
> while it may already be actively queued from the first iteration. Does
> re-initializing an active work_struct without serialization run the risk
> of corrupting the global workqueue list and crashing?

Yeah key should have been a u32 but when doing so the verifier loose
track of its value because it's only taking 4 bytes on the stack and
then it detects an infinite loop for my two iterations, I'll do:

	u32 key __attribute__((aligned(8))) = 0;

So that the verifier can properly track the value in the for loop.

> 
> > +		if (!value)
> > +			return -ENOENT;
> > +		err = bpf_wq_init(&value->work, &work_map, 0);
> > +		if (err)
> > +			return err;
> > +		if (key)
> > +			err = bpf_wq_set_callback(&value->work, ksock_wq_create, 0);
> > +		else
> > +			err = bpf_wq_set_callback(&value->work, ksock_wq_connect_send, 0);
> > +		if (err)
> > +			return err;
> > +		err = bpf_wq_start(&value->work, 0);
> > +		if (err)
> > +			return err;
> > +	}
> > +	return 0;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20261013182726.276372-1-mahe.tardy@gmail.com?part=1