[PATCH] mtd: spi-nor: Fix integer overflow in stacked memories support

[Abbarapu, Venkatesh]

Hi,

> -----Original Message-----
> From: Marek Vasut <marek.vasut+renesas at mailbox.org>
> Sent: Sunday, November 3, 2024 5:28 AM
> To: u-boot at lists.denx.de
> Cc: Marek Vasut <marek.vasut+renesas at mailbox.org>; Heinrich Schuchardt
> <xypron.glpk at gmx.de>; Cédric Le Goater <clg at kaod.org>; Ashok Reddy Soma
> <ashok.reddy.soma at amd.com>; Erkiaga Elorza, Ibai <ibai.erkiaga-
> elorza at amd.com>; Jagan Teki <jagan at amarulasolutions.com>; Simek, Michal
> <michal.simek at amd.com>; Tom Rini <trini at konsulko.com>; Abbarapu, Venkatesh
> <venkatesh.abbarapu at amd.com>; William Zhang <william.zhang at broadcom.com>
> Subject: [PATCH] mtd: spi-nor: Fix integer overflow in stacked memories support
> 
> The 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support")
> adds new SPI bus flags, but also introduces a completely new set of SPI bus flags in
> another location. The existing flags field is type u8, while the new separate flags are
> BIT(8) and higher. Use of those new flags triggers integer overflow.
> 
> Drop the newly introduced flags which were never used anywhere in the code. Move
> the one remaining flag which was used in the correct place and change it from BIT(8)
> to BIT(6) so it fits the u8 flags.
> 
> Fixes: 5d40b3d384dc ("mtd: spi-nor: Add parallel and stacked memories support")
> Addresses-Coverity-ID: 510804 Extra high-order bits
> Reported-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> ---
> Cc: "Cédric Le Goater" <clg at kaod.org>
> Cc: Ashok Reddy Soma <ashok.reddy.soma at amd.com>
> Cc: Heinrich Schuchardt <xypron.glpk at gmx.de>
> Cc: Ibai Erkiaga <ibai.erkiaga-elorza at amd.com>
> Cc: Jagan Teki <jagan at amarulasolutions.com>
> Cc: Michal Simek <michal.simek at amd.com>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com>
> Cc: William Zhang <william.zhang at broadcom.com>
> ---
> Maybe we should revert the whole stacked patchset until it is fixed properly ?
> ---
>  drivers/spi/zynq_qspi.c    | 2 +-
>  drivers/spi/zynqmp_gqspi.c | 6 +++---
>  include/spi.h              | 8 ++------
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c index
> 4aad3248d9e..e43dbb40c4a 100644
> --- a/drivers/spi/zynq_qspi.c
> +++ b/drivers/spi/zynq_qspi.c
> @@ -813,7 +813,7 @@ static int zynq_qspi_exec_op(struct spi_slave *slave,
> 
>  	priv->is_parallel = false;
>  	priv->is_stacked = false;
> -	slave->flags &= ~SPI_XFER_MASK;
> +	slave->flags &= ~SPI_XFER_LOWER;
>  	spi_release_bus(slave);
> 
>  	return 0;
> diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c index
> 1d19b2606c5..4251bf28cd3 100644
> --- a/drivers/spi/zynqmp_gqspi.c
> +++ b/drivers/spi/zynqmp_gqspi.c
> @@ -870,8 +870,8 @@ static int zynqmp_qspi_exec_op(struct spi_slave *slave,
>  	priv->bus = 0;
> 
>  	if (priv->is_parallel) {
> -		if (slave->flags & SPI_XFER_MASK)
> -			priv->bus = (slave->flags & SPI_XFER_MASK) >> 8;
> +		if (slave->flags & SPI_XFER_LOWER)
> +			priv->bus = 1;
>  		if (zynqmp_qspi_update_stripe(op))
>  			priv->stripe = 1;
>  	}
> @@ -890,7 +890,7 @@ static int zynqmp_qspi_exec_op(struct spi_slave *slave,
>  	zynqmp_qspi_chipselect(priv, 0);
> 
>  	priv->is_parallel = false;
> -	slave->flags &= ~SPI_XFER_MASK;
> +	slave->flags &= ~SPI_XFER_LOWER;
> 
>  	return ret;
>  }
> diff --git a/include/spi.h b/include/spi.h index 3a92d02f215..6944773b596 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -41,12 +41,6 @@
>  #define SPI_3BYTE_MODE 0x0
>  #define SPI_4BYTE_MODE 0x1
> 
> -/* SPI transfer flags */
> -#define SPI_XFER_STRIPE	(1 << 6)
> -#define SPI_XFER_MASK	(3 << 8)
> -#define SPI_XFER_LOWER	(1 << 8)
> -#define SPI_XFER_UPPER	(2 << 8)
> -
>  /* Max no. of CS supported per spi device */
>  #define SPI_CS_CNT_MAX	2
> 
> @@ -169,6 +163,8 @@ struct spi_slave {
>  #define SPI_XFER_ONCE		(SPI_XFER_BEGIN | SPI_XFER_END)
>  #define SPI_XFER_U_PAGE		BIT(4)
>  #define SPI_XFER_STACKED	BIT(5)
> +#define SPI_XFER_LOWER		BIT(6)
> +
>  	/*
>  	 * Flag indicating that the spi-controller has multi chip select
>  	 * capability and can assert/de-assert more than one chip select
> --
> 2.45.2


Reviewed-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at amd.com


Thanks
Venkatesh