| Message ID | 20190815083252.GD27238@mwanda |
|---|---|
| State | Accepted |
| Delegated to: | Ambarus Tudor |
| Headers | show |
| Series | mtd: spi-nor: Fix an error code in spi_nor_read_raw() | expand |
Am 15.08.2019 10:32, schrieb Dan Carpenter: > The problem is that if "ret" is negative then when we check if > "ret > len", that condition is going to be true because of type > promotion. So this patch re-orders the code to check for negatives > first and preserve those error codes. > > Fixes: f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/mtd/spi-nor/spi-nor.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 63af87609bac..986b0754495d 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf) > > while (len) { > ret = spi_nor_read_data(nor, addr, len, buf); > - if (!ret || ret > len) > - return -EIO; > if (ret < 0) > return ret; > + if (!ret || ret > len) > + return -EIO; Bonuspoints to make this more readable: if (ret==0 || ret > len) return -EIO; that makes the intention more obvious. JM2C, wh > > buf += ret; > addr += ret;
On Thu, Aug 15, 2019 at 12:35:49PM +0200, walter harms wrote: > > > Am 15.08.2019 10:32, schrieb Dan Carpenter: > > The problem is that if "ret" is negative then when we check if > > "ret > len", that condition is going to be true because of type > > promotion. So this patch re-orders the code to check for negatives > > first and preserve those error codes. > > > > Fixes: f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables") > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > > --- > > drivers/mtd/spi-nor/spi-nor.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > > index 63af87609bac..986b0754495d 100644 > > --- a/drivers/mtd/spi-nor/spi-nor.c > > +++ b/drivers/mtd/spi-nor/spi-nor.c > > @@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf) > > > > while (len) { > > ret = spi_nor_read_data(nor, addr, len, buf); > > - if (!ret || ret > len) > > - return -EIO; > > if (ret < 0) > > return ret; > > + if (!ret || ret > len) > > + return -EIO; > > Bonuspoints to make this more readable: > > if (ret==0 || ret > len) > return -EIO; > > that makes the intention more obvious. That's not really related to the bug fix. I do agree with your style though. "ret" is a number here, and not a bool or error vs no error so == 0 is nicer. regards, dan carpenter
On 08/15/2019 11:32 AM, Dan Carpenter wrote: > External E-Mail > > > The problem is that if "ret" is negative then when we check if > "ret > len", that condition is going to be true because of type > promotion. So this patch re-orders the code to check for negatives > first and preserve those error codes. > > Fixes: f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by: Tudor Ambarus <tudor.ambarus@microchip.com> > --- > drivers/mtd/spi-nor/spi-nor.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c > index 63af87609bac..986b0754495d 100644 > --- a/drivers/mtd/spi-nor/spi-nor.c > +++ b/drivers/mtd/spi-nor/spi-nor.c > @@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf) > > while (len) { > ret = spi_nor_read_data(nor, addr, len, buf); > - if (!ret || ret > len) > - return -EIO; > if (ret < 0) > return ret; > + if (!ret || ret > len) > + return -EIO; > > buf += ret; > addr += ret; >
On 08/15/2019 11:32 AM, Dan Carpenter wrote: > External E-Mail > > > The problem is that if "ret" is negative then when we check if > "ret > len", that condition is going to be true because of type > promotion. So this patch re-orders the code to check for negatives > first and preserve those error codes. > > Fixes: f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > --- > drivers/mtd/spi-nor/spi-nor.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Since the bug was not introduced in the previous release and we are quite late for mtd/fixes, Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git, spi-nor/next branch. Thanks, ta
diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index 63af87609bac..986b0754495d 100644 --- a/drivers/mtd/spi-nor/spi-nor.c +++ b/drivers/mtd/spi-nor/spi-nor.c @@ -2903,10 +2903,10 @@ static int spi_nor_read_raw(struct spi_nor *nor, u32 addr, size_t len, u8 *buf) while (len) { ret = spi_nor_read_data(nor, addr, len, buf); - if (!ret || ret > len) - return -EIO; if (ret < 0) return ret; + if (!ret || ret > len) + return -EIO; buf += ret; addr += ret;
The problem is that if "ret" is negative then when we check if "ret > len", that condition is going to be true because of type promotion. So this patch re-orders the code to check for negatives first and preserve those error codes. Fixes: f384b352cbf0 ("mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/mtd/spi-nor/spi-nor.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)