linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2 v4] spi: rspi: Fixes bogus received byte and replaces "n" by "len"
@ 2017-02-15 10:50 DongCV
       [not found] ` <1487155852-12102-1-git-send-email-cv-dong-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
  0 siblings, 1 reply; 6+ messages in thread
From: DongCV @ 2017-02-15 10:50 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ,
	yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ,
	ryusuke.sakato.bx-zM6kxYcvzFBBDgjK7y7TUQ,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	nv-dung-HEF513clHfp3+QwDJ9on6Q, h-inayoshi-HEF513clHfp3+QwDJ9on6Q,
	cm-hiep-HEF513clHfp3+QwDJ9on6Q

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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in()
       [not found] ` <1487155852-12102-1-git-send-email-cv-dong-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
@ 2017-02-15 10:50   ` DongCV
  2017-02-15 12:17     ` Sergei Shtylyov
  2017-02-15 10:50   ` [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*() DongCV
  1 sibling, 1 reply; 6+ messages in thread
From: DongCV @ 2017-02-15 10:50 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ,
	yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ,
	ryusuke.sakato.bx-zM6kxYcvzFBBDgjK7y7TUQ,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	nv-dung-HEF513clHfp3+QwDJ9on6Q, h-inayoshi-HEF513clHfp3+QwDJ9on6Q,
	cm-hiep-HEF513clHfp3+QwDJ9on6Q

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-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*()
       [not found] ` <1487155852-12102-1-git-send-email-cv-dong-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
  2017-02-15 10:50   ` [PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in() DongCV
@ 2017-02-15 10:50   ` DongCV
  2017-02-15 12:17     ` Geert Uytterhoeven
  2017-02-16 19:05     ` Applied "spi: rspi: Replaces "n" by "len" in qspi_transfer_*()" to the spi tree Mark Brown
  1 sibling, 2 replies; 6+ messages in thread
From: DongCV @ 2017-02-15 10:50 UTC (permalink / raw)
  To: broonie-DgEjT+Ai2ygdnm+yROfE0A,
	geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ,
	linux-spi-u79uwXL29TY76Z2rM5mHXA
  Cc: kuninori.morimoto.gx-zM6kxYcvzFBBDgjK7y7TUQ,
	yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ,
	ryusuke.sakato.bx-zM6kxYcvzFBBDgjK7y7TUQ,
	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
	nv-dung-HEF513clHfp3+QwDJ9on6Q, h-inayoshi-HEF513clHfp3+QwDJ9on6Q,
	cm-hiep-HEF513clHfp3+QwDJ9on6Q

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-HEF513clHfp3+QwDJ9on6Q@public.gmane.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 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

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*()
  2017-02-15 10:50   ` [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*() DongCV
@ 2017-02-15 12:17     ` Geert Uytterhoeven
  2017-02-16 19:05     ` Applied "spi: rspi: Replaces "n" by "len" in qspi_transfer_*()" to the spi tree Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2017-02-15 12:17 UTC (permalink / raw)
  To: DongCV
  Cc: Mark Brown, Geert Uytterhoeven, linux-spi, Kuninori Morimoto,
	Yoshihiro Shimoda, Ryusuke Sakato, Linux-Renesas,
	Dung:人ソ, 稲吉, Cao Minh Hiep

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in()
  2017-02-15 10:50   ` [PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in() DongCV
@ 2017-02-15 12:17     ` Sergei Shtylyov
  0 siblings, 0 replies; 6+ messages in thread
From: Sergei Shtylyov @ 2017-02-15 12:17 UTC (permalink / raw)
  To: DongCV, broonie, geert+renesas, linux-spi
  Cc: kuninori.morimoto.gx, yoshihiro.shimoda.uh, ryusuke.sakato.bx,
	linux-renesas-soc, nv-dung, h-inayoshi, cm-hiep

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Applied "spi: rspi: Replaces "n" by "len" in qspi_transfer_*()" to the spi tree
  2017-02-15 10:50   ` [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*() DongCV
  2017-02-15 12:17     ` Geert Uytterhoeven
@ 2017-02-16 19:05     ` Mark Brown
  1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2017-02-16 19:05 UTC (permalink / raw)
  To: DongCV
  Cc: Mark Brown, broonie, geert+renesas, linux-spi,
	kuninori.morimoto.gx, yoshihiro.shimoda.uh, ryusuke.sakato.bx,
	linux-renesas-soc, nv-dung, h-inayoshi, cm-hiep, linux-spi

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-02-16 19:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-15 10:50 [PATCH 0/2 v4] spi: rspi: Fixes bogus received byte and replaces "n" by "len" DongCV
     [not found] ` <1487155852-12102-1-git-send-email-cv-dong-HEF513clHfp3+QwDJ9on6Q@public.gmane.org>
2017-02-15 10:50   ` [PATCH 1/2] spi: rspi: Fixes bogus received byte in qspi_transfer_in() DongCV
2017-02-15 12:17     ` Sergei Shtylyov
2017-02-15 10:50   ` [PATCH 2/2] spi: rspi: Replaces "n" by "len" in qspi_transfer_*() DongCV
2017-02-15 12:17     ` Geert Uytterhoeven
2017-02-16 19:05     ` Applied "spi: rspi: Replaces "n" by "len" in qspi_transfer_*()" to the spi tree Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).