[PATCH 0/2 v4] spi: rspi: Fixes bogus received byte and replaces "n" by "len"

[PATCH 0/2 v4] spi: rspi: Fixes bogus received byte and replaces "n" by "len"

[DongCV]

Hi Mark,
Cc: Geert, Sergei

Thanks for your comments and advices.

I separated my patch into two patches as you've shown me.
These patches fix: 3be09bec42a800d4 "spi: rspi: supports 32bytes 
buffer for DUAL and QUAD". One of them is to fix bogus received byte 
into qspi_transfer_in(), and other is to replace "n" by "len" bytes of 
data in qspi_transfer_in()and qspi_transfer_out().
These patches were tested at v4.10-rc8 on Renesas Gen2 Lager board.

Please consider these patches to Renesas Rcar family.
Thank you.
Jinso/Dong.

DongCV (2):
  spi: rspi: Fixes bogus received byte in qspi_transfer_in()
  spi: rspi: Replaces "n" by "len" in qspi_transfer_in() and
    qspi_transfer_out()

 drivers/spi/spi-rspi.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

-- 
1.9.1

[PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in()

[DongCV]

In qspi_transfer_in(), when receiving the last n (or len) bytes of data,
one bogus byte was written in the receive buffer.
This code leads to a buffer overflow.

"jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found 
at 0x03b40000: 0x1900 instead
jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found 
at 0x03b40004: 0x000c instead"

The error message above happens when trying to mount, unmount,
and remount a jffs2-formatted device.
This patch removed the bogus write to fixes: 3be09bec42a800d4
"spi: rspi: supports 32bytes buffer for DUAL and QUAD"

And here is Geert's comment:

"spi: rspi: Fix bogus received byte in qspi_transfer_in()
When there are less than QSPI_BUFFER_SIZE remaining bytes to be received,
qspi_transfer_in() writes one bogus byte in the receive buffer, possibly
leading to a buffer overflow.
This can be reproduced by mounting, unmounting, and remounting a
jffs2-formatted device, causing lots of warnings like:

"jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found
at 0x03b40000: 0x1900 instead"

Remove the bogus write to fix this. "

Signed-off-by: DongCV <cv-dong@jinso.co.jp>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 drivers/spi/spi-rspi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 9daf500..2ee1301 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -848,7 +848,6 @@ static int qspi_transfer_in(struct rspi_data *rspi, struct spi_transfer *xfer)
 			ret = rspi_pio_transfer(rspi, NULL, rx, n);
 			if (ret < 0)
 				return ret;
-			*rx++ = ret;
 		}
 		n -= len;
 	}
-- 
1.9.1

Re: [PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in()

[Sergei Shtylyov]

On 02/15/2017 01:50 PM, DongCV wrote:

> In qspi_transfer_in(), when receiving the last n (or len) bytes of data,
> one bogus byte was written in the receive buffer.
> This code leads to a buffer overflow.
>
> "jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found
> at 0x03b40000: 0x1900 instead
> jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found
> at 0x03b40004: 0x000c instead"
>
> The error message above happens when trying to mount, unmount,
> and remount a jffs2-formatted device.
> This patch removed the bogus write to fixes: 3be09bec42a800d4
> "spi: rspi: supports 32bytes buffer for DUAL and QUAD"

    You were just asked to add the following tag to the patch (e.g. before 
your signoff):

Fixes: 3be09bec42a8 ("spi: rspi: supports 32bytes buffer for DUAL and QUAD")

This simplifies the propagation of the patch to the -stable releases...

> And here is Geert's comment:
>
> "spi: rspi: Fix bogus received byte in qspi_transfer_in()
> When there are less than QSPI_BUFFER_SIZE remaining bytes to be received,
> qspi_transfer_in() writes one bogus byte in the receive buffer, possibly
> leading to a buffer overflow.
> This can be reproduced by mounting, unmounting, and remounting a
> jffs2-formatted device, causing lots of warnings like:
>
> "jffs2: jffs2_scan_eraseblock(): Magic bitmask 0x1985 not found
> at 0x03b40000: 0x1900 instead"
>
> Remove the bogus write to fix this. "

    I don't think effectively duplicating your patch description makes sense here.

> Signed-off-by: DongCV <cv-dong@jinso.co.jp>

    Need full name here.

> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
[...]

MBR, Sergei

[PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*()

[DongCV]

This patch replaced "n" by "len" bytes of data in qspi_transfer_in() and
qspi_transfer_out() function. This will make improving readability.

Signed-off-by: DongCV <cv-dong@jinso.co.jp>
---
 drivers/spi/spi-rspi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 2ee1301..bc3c868 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -808,7 +808,7 @@ static int qspi_transfer_out(struct rspi_data *rspi, struct spi_transfer *xfer)
 			for (i = 0; i < len; i++)
 				rspi_write_data(rspi, *tx++);
 		} else {
-			ret = rspi_pio_transfer(rspi, tx, NULL, n);
+			ret = rspi_pio_transfer(rspi, tx, NULL, len);
 			if (ret < 0)
 				return ret;
 		}
@@ -845,7 +845,7 @@ static int qspi_transfer_in(struct rspi_data *rspi, struct spi_transfer *xfer)
 			for (i = 0; i < len; i++)
 				*rx++ = rspi_read_data(rspi);
 		} else {
-			ret = rspi_pio_transfer(rspi, NULL, rx, n);
+			ret = rspi_pio_transfer(rspi, NULL, rx, len);
 			if (ret < 0)
 				return ret;
 		}
-- 
1.9.1

Re: [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*()

[Geert Uytterhoeven]

On Wed, Feb 15, 2017 at 11:50 AM, DongCV <cv-dong@jinso.co.jp> wrote:
> This patch replaced "n" by "len" bytes of data in qspi_transfer_in() and
> qspi_transfer_out() function. This will make improving readability.
>
> Signed-off-by: DongCV <cv-dong@jinso.co.jp>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Applied "spi: rspi: Replaces "n" by "len" in qspi_transfer_*()" to the spi tree

[Mark Brown]

The patch

   spi: rspi: Replaces "n" by "len" in qspi_transfer_*()

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From ad16d4a83ddc86151b4a6efe257ba74eb30f9f8e Mon Sep 17 00:00:00 2001
From: DongCV <cv-dong@jinso.co.jp>
Date: Wed, 15 Feb 2017 19:50:52 +0900
Subject: [PATCH] spi: rspi: Replaces "n" by "len" in qspi_transfer_*()

This patch replaced "n" by "len" bytes of data in qspi_transfer_in() and
qspi_transfer_out() function. This will make improving readability.

Signed-off-by: DongCV <cv-dong@jinso.co.jp>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/spi/spi-rspi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-rspi.c b/drivers/spi/spi-rspi.c
index 2ee130138066..bc3c8686f4d9 100644
--- a/drivers/spi/spi-rspi.c
+++ b/drivers/spi/spi-rspi.c
@@ -808,7 +808,7 @@ static int qspi_transfer_out(struct rspi_data *rspi, struct spi_transfer *xfer)
 			for (i = 0; i < len; i++)
 				rspi_write_data(rspi, *tx++);
 		} else {
-			ret = rspi_pio_transfer(rspi, tx, NULL, n);
+			ret = rspi_pio_transfer(rspi, tx, NULL, len);
 			if (ret < 0)
 				return ret;
 		}
@@ -845,7 +845,7 @@ static int qspi_transfer_in(struct rspi_data *rspi, struct spi_transfer *xfer)
 			for (i = 0; i < len; i++)
 				*rx++ = rspi_read_data(rspi);
 		} else {
-			ret = rspi_pio_transfer(rspi, NULL, rx, n);
+			ret = rspi_pio_transfer(rspi, NULL, rx, len);
 			if (ret < 0)
 				return ret;
 		}
-- 
2.11.0