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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: selftests: u64 map key mismatch causes ksock_wq failure on big-endian
--

commit e40a8841b185289348c211758c8754c0959c28bd
Author: Mahe Tardy <mahe.tardy@gmail.com>

bpf: allow ksock connect/send calls from wq

This commit relaxes the constraint on not using the workqueues for ksock 
connect() or send() kfuncs. It also updates the selftests to verify this 
behavior.

> 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
> @@ -14,15 +15,19 @@ struct ksock_wq_value {
>  
>  struct {
>  	__uint(type, BPF_MAP_TYPE_ARRAY);
> -	__uint(max_entries, 1);
> +	__uint(max_entries, 2);
>  	__type(key, u32);
>  	__type(value, struct ksock_wq_value);
>  } work_map SEC(".maps");

[ ... ]

> @@ -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]
Could this cause failures on big-endian systems?

In ksock_wq_start(), the key variable was changed from u32 to u64, but
work_map is defined as a BPF_MAP_TYPE_ARRAY with a u32 key.

When &key is passed to bpf_map_lookup_elem(), big-endian systems will read
the first 4 bytes of the u64 (which will be 0 for both iterations in the
loop).

This would cause the lookup to return the element at index 0 multiple times,
leading to double-initialization of the same workqueue via bpf_wq_init()
and subsequent failures.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014091450.19107-1-mahe.tardy@gmail.com?part=1