Re: [PATCH net v2 2/2] selftests: net: add kTLS async split record regression

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Memory and file descriptor leak of `struct bpf_object` on error paths inside `attach_bpf()`.
- [Low] Including libbpf source headers directly instead of installed/exported headers in Makefile.
--

commit 4256015d1c1b459bffc8e935d002cdb7f0465799
Author: Christopher Lusk <clusk@northecho.dev>

selftests: net: add kTLS async split record regression

Add a kTLS TX regression test for the BPF sk_msg apply_bytes split-record
path. The test sends four 4096-byte writes through a sockmap verdict program
that applies 512-byte verdict windows, then verifies that the peer receives
the full TLS-protected stream.

> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
> index f3da38c54d276..0435327e93a72 100644
> --- a/tools/testing/selftests/net/Makefile
> +++ b/tools/testing/selftests/net/Makefile
[ ... ]
> @@ -236,3 +237,7 @@ $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
>  $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
>  
>  include bpf.mk
> +
> +$(OUTPUT)/ktls_async_split: CFLAGS += -I../../../lib

[Severity: Low]
Is this include path correct? By pointing directly to lib instead of the
exported headers like ../../../include/, could we bypass the proper libbpf
build and export step and rely on raw source headers instead?

> diff --git a/tools/testing/selftests/net/ktls_async_split.c b/tools/testing/selftests/net/ktls_async_split.c
> new file mode 100644
> index 0000000000000..5601ce4b5f688
> --- /dev/null
> +++ b/tools/testing/selftests/net/ktls_async_split.c
[ ... ]
> +static int attach_bpf(const char *obj_path, int sock_fd, struct bpf_object **obj_out)
> +{
> +	struct bpf_object *obj;
> +	struct bpf_program *prog;
> +	int map_fd, prog_fd;
> +	__u32 key = 0;
> +
> +	obj = bpf_object__open_file(obj_path, NULL);
> +	if (libbpf_get_error(obj))
> +		return -EINVAL;
> +	if (bpf_object__load(obj))
> +		return -EINVAL;

[Severity: Medium]
Does this leak the bpf_object structure and associated resources on error
paths?

Since *obj_out is only assigned at the end of the function upon success, if
bpf_object__load() or any subsequent operations fail, the caller receives a
NULL pointer and cannot clean up the leaked object via bpf_object__close().

> +
> +	prog = bpf_object__find_program_by_name(obj, "apply_bytes_verdict");
> +	if (!prog)
> +		return -ENOENT;
> +	prog_fd = bpf_program__fd(prog);
> +	if (prog_fd < 0)
> +		return prog_fd;
> +
> +	map_fd = bpf_object__find_map_fd_by_name(obj, "sock_map");
> +	if (map_fd < 0)
> +		return -ENOENT;
> +
> +	if (bpf_prog_attach(prog_fd, map_fd, BPF_SK_MSG_VERDICT, 0))
> +		return -errno;
> +	if (bpf_map_update_elem(map_fd, &key, &sock_fd, BPF_ANY))
> +		return -errno;
> +
> +	*obj_out = obj;
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521025840.976378-1-clusk@northecho.dev?part=2