Re: [PATCH net-next v3 02/13] net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The `size` parameter in `oa_tc6_check_ctrl_write_reply()` is typed as `u8`, causing integer truncation for SPI transfer sizes of 256 bytes or more. This leads to an underflow in the `memcmp` length calculation, resulting in a massive out-of-bounds memory read and kernel panic.
--

commit afec089ed8ebeec6d0ad251fec33bc20cdb36a86
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode

This commit implements the OA TC6 standard defined protected mode for
control transactions. It handles 1's complement values of the data field
in SPI transfer frames for integrity checking.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 91a906a7918a2..baba5aad84dfa 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c

[ ... ]

> @@ -283,10 +320,10 @@ static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
>  	u16 size;
>  	int ret;
>  
> -	/* Prepare control command and copy to SPI control buffer */
> -	oa_tc6_prepare_ctrl_spi_buf(tc6, address, value, length, reg_op);
> +	size = oa_tc6_calculate_ctrl_buf_size(length, tc6->prot_ctrl);
>  
> -	size = oa_tc6_calculate_ctrl_buf_size(length);
> +	/* Prepare control command and copy to SPI control buffer */
> +	oa_tc6_prepare_ctrl_spi_buf(tc6, address, value, length, reg_op, size);
>  
>  	/* Perform SPI transfer */
>  	ret = oa_tc6_spi_transfer(tc6, OA_TC6_CTRL_HEADER, size);

[ ... ]

> @@ -301,7 +338,7 @@ static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
>  		return oa_tc6_check_ctrl_write_reply(tc6, size);

[Severity: High]
This is a pre-existing issue, but does passing the u16 size to
oa_tc6_check_ctrl_write_reply() lead to an integer truncation here?

Looking at the function signature, oa_tc6_check_ctrl_write_reply() takes
size as a u8:

static int oa_tc6_check_ctrl_write_reply(struct oa_tc6 *tc6, u8 size)

Since oa_tc6_calculate_ctrl_buf_size() computes a size up to 1032 bytes in
protected mode, any value of 256 or more will be truncated when implicitly
cast to a u8.

If a write sequence results in a buffer size that truncates to a value
less than OA_TC6_CTRL_IGNORED_SIZE (4 bytes), could the subtraction in
the integrity check underflow?

static int oa_tc6_check_ctrl_write_reply(struct oa_tc6 *tc6, u8 size)
{
        ...
        if (memcmp(tx_buf, rx_buf, size - OA_TC6_CTRL_IGNORED_SIZE))
                return -EPROTO;
}

For example, if the calculated size is exactly 256 bytes (which can
happen when writing 31 registers in protected mode), the truncated
size becomes 0.

The subtraction 0 - 4 results in -4, which becomes SIZE_MAX - 3 when
passed as the size_t parameter to memcmp(), resulting in an
out-of-bounds read and likely a kernel panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604-adin1140-driver-v3-0-5debdb3173c4@analog.com?part=2