Re: [PATCH net-next v5 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] Integer truncation in `oa_tc6_check_ctrl_write_reply()` causes out-of-bounds read and kernel panic
--

--- Patch [2]: [PATCH net-next v5 02/13] net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode ---
commit f765aab54c97488541b3e9e738aa5de9bff2269a
Author: Ciprian Regus <ciprian.regus@analog.com>

net: ethernet: oa_tc6: Handle the OA TC6 SPI protected mode
    
This patch implements the OA TC6 standard defined protected mode for control
(register access) transactions. In addition to the current register access
formats the oa_tc6 driver handles, 1's complement values of the data field
are included in the SPI transfer frames as an integrity check.

> diff --git a/drivers/net/ethernet/oa_tc6.c b/drivers/net/ethernet/oa_tc6.c
> index 0727d53345a33..8b96558834960 100644
> --- a/drivers/net/ethernet/oa_tc6.c
> +++ b/drivers/net/ethernet/oa_tc6.c
[ ... ]
> @@ -284,10 +321,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);

[ ... ]

> @@ -302,7 +339,7 @@ static int oa_tc6_perform_ctrl(struct oa_tc6 *tc6, u32 address, u32 value[],
>  		return oa_tc6_check_ctrl_write_reply(tc6, size);
>  
>  	/* Check echoed/received control read command reply for errors */
> -	ret = oa_tc6_check_ctrl_read_reply(tc6, size);
> +	ret = oa_tc6_check_ctrl_read_reply(tc6, length);

[Severity: High]
This is a pre-existing issue, but does oa_tc6_check_ctrl_write_reply() safely
handle the u16 size being passed to it?

The size variable is declared as a u16 in oa_tc6_perform_ctrl(), but
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)
{
	...
	if (memcmp(tx_buf, rx_buf, size - OA_TC6_CTRL_IGNORED_SIZE))
		return -EPROTO;
	...
}

If size is exactly 256 bytes (which can happen when writing 62 registers in
unprotected mode, or 31 in protected mode), the size parameter truncates to 0.

The subtraction size - OA_TC6_CTRL_IGNORED_SIZE then becomes 0 - 4 = -4.
When cast to size_t for memcmp(), this becomes a massive out-of-bounds read
that can cause a page fault and kernel panic.

Additionally, if the truncated size is greater than 0 but less than the actual
size, memcmp() will only compare a few bytes, silently bypassing the full
integrity check.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-adin1140-driver-v5-0-4aca7b51a58b@analog.com?part=2