* Re: [PATCH 1/1] net: ax88796c: do not receive data in pointer
2021-11-21 20:06 ` [PATCH 1/1] net: ax88796c: do not receive data in pointer Nicolas Iooss
@ 2021-11-22 13:52 ` Lukasz Stelmach
2021-11-22 14:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Lukasz Stelmach @ 2021-11-22 13:52 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: David S. Miller, Jakub Kicinski, netdev, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2687 bytes --]
It was <2021-11-21 nie 21:06>, when Nicolas Iooss wrote:
> Function axspi_read_status calls:
>
> ret = spi_write_then_read(ax_spi->spi, ax_spi->cmd_buf, 1,
> (u8 *)&status, 3);
>
> status is a pointer to a struct spi_status, which is 3-byte wide:
>
> struct spi_status {
> u16 isr;
> u8 status;
> };
>
> But &status is the pointer to this pointer, and spi_write_then_read does
> not dereference this parameter:
>
> int spi_write_then_read(struct spi_device *spi,
> const void *txbuf, unsigned n_tx,
> void *rxbuf, unsigned n_rx)
>
> Therefore axspi_read_status currently receive a SPI response in the
> pointer status, which overwrites 24 bits of the pointer.
>
> Thankfully, on Little-Endian systems, the pointer is only used in
>
> le16_to_cpus(&status->isr);
>
> ... which is a no-operation. So there, the overwritten pointer is not
> dereferenced. Nevertheless on Big-Endian systems, this can lead to
> dereferencing pointers after their 24 most significant bits were
> overwritten. And in all systems this leads to possible use of
> uninitialized value in functions calling spi_write_then_read which
> expect status to be initialized when the function returns.
>
> Moreover function axspi_read_status (and macro AX_READ_STATUS) do not
> seem to be used anywhere. So currently this seems to be dead code. Fix
> the issue anyway so that future code works properly when using function
> axspi_read_status.
>
Thank you for spotting and fixing this one.
> Fixes: a97c69ba4f30 ("net: ax88796c: ASIX AX88796C SPI Ethernet Adapter Driver")
>
> Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
> ---
> drivers/net/ethernet/asix/ax88796c_spi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Łukasz Stelmach <l.stelmach@samsung.com>
> diff --git a/drivers/net/ethernet/asix/ax88796c_spi.c b/drivers/net/ethernet/asix/ax88796c_spi.c
> index 94df4f96d2be..0710e716d682 100644
> --- a/drivers/net/ethernet/asix/ax88796c_spi.c
> +++ b/drivers/net/ethernet/asix/ax88796c_spi.c
> @@ -34,7 +34,7 @@ int axspi_read_status(struct axspi_data *ax_spi, struct spi_status *status)
>
> /* OP */
> ax_spi->cmd_buf[0] = AX_SPICMD_READ_STATUS;
> - ret = spi_write_then_read(ax_spi->spi, ax_spi->cmd_buf, 1, (u8 *)&status, 3);
> + ret = spi_write_then_read(ax_spi->spi, ax_spi->cmd_buf, 1, (u8 *)status, 3);
> if (ret)
> dev_err(&ax_spi->spi->dev, "%s() failed: ret = %d\n", __func__, ret);
> else
--
Łukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] net: ax88796c: do not receive data in pointer
2021-11-21 20:06 ` [PATCH 1/1] net: ax88796c: do not receive data in pointer Nicolas Iooss
2021-11-22 13:52 ` Lukasz Stelmach
@ 2021-11-22 14:40 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-11-22 14:40 UTC (permalink / raw)
To: Nicolas Iooss; +Cc: l.stelmach, davem, kuba, netdev, linux-kernel
Hello:
This patch was applied to netdev/net.git (master)
by David S. Miller <davem@davemloft.net>:
On Sun, 21 Nov 2021 21:06:42 +0100 you wrote:
> Function axspi_read_status calls:
>
> ret = spi_write_then_read(ax_spi->spi, ax_spi->cmd_buf, 1,
> (u8 *)&status, 3);
>
> status is a pointer to a struct spi_status, which is 3-byte wide:
>
> [...]
Here is the summary with links:
- [1/1] net: ax88796c: do not receive data in pointer
https://git.kernel.org/netdev/net/c/f93fd0ca5e7d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread