Re: [PATCH] selftests: can: add CAN XL raw socket tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kselftests/can: missing XL capability check causes test failure
--

commit 0db65e488a9ffaf096720dc302fd90d7dbdccbc6
Author: Quchaosheng <quchaosheng000406@163.com>

selftests: can: add CAN XL raw socket tests

This commit adds a set of CAN XL tests to `test_raw_filter.c` utilizing a
vcan interface. It tests payload round-trips, frame validation, CAN FD
dependencies, and VCID filtering.

> diff --git a/tools/testing/selftests/net/can/test_raw_filter.c b/tools/testing/selftests/net/can/test_raw_filter.c
> index bb8ae88542737..53f0c9d829ed0 100644
> --- a/tools/testing/selftests/net/can/test_raw_filter.c
> +++ b/tools/testing/selftests/net/can/test_raw_filter.c

[ ... ]

> +static int open_xl_socket(int send_xl, const struct can_raw_vcid_options *vcid)
> +{
> +	struct sockaddr_can addr;
> +	struct ifreq ifr;
> +	int one = 1;
> +	int s, ret;

[ ... ]

> +	addr.can_family = AF_CAN;
> +	addr.can_ifindex = ifr.ifr_ifindex;
> +
> +	ret = bind(s, (struct sockaddr *)&addr, sizeof(addr));

[Severity: Low]
This is a pre-existing issue, but does passing this struct to bind() leak
uninitialized stack memory to the kernel?

The addr struct has padding and a large union (can_addr) that are not fully
initialized by setting just can_family and can_ifindex. Could this trigger
memory sanitizer warnings when sizeof(addr) bytes are copied across the
user/kernel boundary?

[ ... ]

> +/* Without VCID options the VCID is stripped on TX and a VCID-tagged frame is
> + * not forwarded on RX
> + */
> +TEST(canxl_vcid_stripped_by_default)
> +{
> +	struct can_raw_vcid_options pass = {
> +		.flags = CAN_RAW_XL_VCID_TX_PASS,
> +	};
> +	struct canxl_frame tx, rx;
> +	int tx_sock, rx_sock, ret;
> +
> +	tx_sock = open_xl_socket(1, &pass);
> +	ASSERT_GE(tx_sock, 0)
> +		TH_LOG("CAN XL not supported on %s", CANIF);
> +
> +	rx_sock = open_xl_socket(1, NULL);
> +	ASSERT_GE(rx_sock, 0);
> +
> +	fill_canxl(&tx, 8, 0x11);
> +	tx.prio |= 0x42 << CANXL_VCID_OFFSET;
> +
> +	ret = write(tx_sock, &tx, CANXL_HDR_SIZE + tx.len);
> +	ASSERT_EQ(ret, CANXL_HDR_SIZE + tx.len);

[Severity: Medium]
Does this test lack a check for CAN XL support before executing?

Other tests in this file check if the interface can handle CAN XL frames:

	if (!can_iface_supports_xl())
		SKIP(return, "%s does not support CAN XL", CANIF);

Without this check, if the tests are run on a system with only a classic CAN
interface, the write() will be rejected with -EINVAL because the MTU is too
small. Won't this cause a false positive test failure at the ASSERT_EQ check
instead of gracefully skipping the test?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923025317.374997-1-quchaosheng000406@163.com?part=1