| Message ID | 20241102235754.74661-1-marek.vasut+renesas@mailbox.org |
|---|---|
| State | New |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
| Series | mtd: spi-nor: Fix integer overflow in stacked memories support | expand |
Hi, > -----Original Message----- > From: Marek Vasut <marek.vasut+renesas@mailbox.org> > Sent: Sunday, November 3, 2024 5:28 AM > To: u-boot@lists.denx.de > Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>; Heinrich Schuchardt > <xypron.glpk@gmx.de>; Cédric Le Goater <clg@kaod.org>; Ashok Reddy Soma > <ashok.reddy.soma@amd.com>; Erkiaga Elorza, Ibai <ibai.erkiaga- > elorza@amd.com>; Jagan Teki <jagan@amarulasolutions.com>; Simek, Michal > <michal.simek@amd.com>; Tom Rini <trini@konsulko.com>; Abbarapu, Venkatesh > <venkatesh.abbarapu@amd.com>; William Zhang <william.zhang@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@gmx.de> > Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org> > --- > Cc: "Cédric Le Goater" <clg@kaod.org> > Cc: Ashok Reddy Soma <ashok.reddy.soma@amd.com> > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> > Cc: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com> > Cc: Jagan Teki <jagan@amarulasolutions.com> > Cc: Michal Simek <michal.simek@amd.com> > Cc: Tom Rini <trini@konsulko.com> > Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> > Cc: William Zhang <william.zhang@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@amd.com Thanks Venkatesh
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
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@gmx.de> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org> --- Cc: "Cédric Le Goater" <clg@kaod.org> Cc: Ashok Reddy Soma <ashok.reddy.soma@amd.com> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de> Cc: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com> Cc: Jagan Teki <jagan@amarulasolutions.com> Cc: Michal Simek <michal.simek@amd.com> Cc: Tom Rini <trini@konsulko.com> Cc: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com> Cc: William Zhang <william.zhang@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(-)