Re: [RFC PATCH v7 2/4] selftests/bpf: add iocost cost model test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: endianness bug when writing 64-bit device ID to 32-bit dev field
--

commit d5f4da7dcc6342f2064a4e05e68659fc439deaa1
Author: Tao Cui <cuitao@kylinos.cn>

selftests/bpf: add iocost cost model test

Add an example cost model implementing the full builtin linear HDD
formula at double cost, and a test which attaches it to one device:
the dev member of the struct_ops is written through the map's
initial value before load, as hid_bpf tests do with hid_id, and
attaching the struct_ops attaches the model to the device.

> diff --git a/tools/testing/selftests/bpf/prog_tests/iocost_model.c b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> new file mode 100644
> index 0000000000000..156c75367af05
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/iocost_model.c
> @@ -0,0 +1,182 @@
[ ... ]
> +void serial_test_iocost_model(void)
> +{
> +	struct iocost_model *skel, *second;
> +	unsigned int maj, min;
> +	__u64 *ops_dev, *sdev;
> +	int err;
> +	char model[32], *dev;
[ ... ]
> +	/* dev is the first member of struct iocost_model_ops */
> +	ops_dev = bpf_map__initial_value(skel->maps.iocost_2x, NULL);
> +	if (!ASSERT_OK_PTR(ops_dev, "initial_value")) {
> +		iocost_model__destroy(skel);
> +		return;
> +	}
> +	*ops_dev = makedev(maj, min);

[Severity: Medium]
Will this 64-bit write cause an endianness issue on big-endian architectures?

Since the first member dev of struct iocost_model_ops is a 32-bit dev_t,
casting the pointer to __u64 * and writing a 64-bit value will place the
actual device ID into the trailing padding on big-endian systems, leaving the
dev field as 0. Could this cause the test to fail by attaching to the wrong
device or being rejected by the kernel?

[ ... ]
> +		/* a second model on the same device fails with -EBUSY */
> +		second = iocost_model__open();
> +		if (ASSERT_OK_PTR(second, "second_open")) {
> +			sdev = bpf_map__initial_value(
> +					second->maps.iocost_2x, NULL);
> +			if (!ASSERT_OK_PTR(sdev, "second_initial_value"))
> +				goto out_destroy;
> +			*sdev = makedev(maj, min);

[Severity: Medium]
Does this have the same 64-bit write endianness issue on big-endian systems
as the first model?

[ ... ]
> +void serial_test_iocost_model_streams(void)
> +{
> +	struct iocost_ms *skel;
> +	unsigned int maj, min;
> +	__u64 *ops_dev;
> +	int err;
> +	char model[32], *dev;
[ ... ]
> +	ops_dev = bpf_map__initial_value(skel->maps.iocost_ms, NULL);
> +	if (!ASSERT_OK_PTR(ops_dev, "initial_value")) {
> +		iocost_ms__destroy(skel);
> +		return;
> +	}
> +	*ops_dev = makedev(maj, min);

[Severity: Medium]
Does this also share the same 64-bit write endianness issue for the 32-bit
dev field?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924054549.2271705-1-cui.tao@linux.dev?part=2