All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
@ 2015-06-15  6:59 Andrew Gabbasov
       [not found] ` <1434351560-29463-1-git-send-email-andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-15  6:59 UTC (permalink / raw)
  To: linux-spi-u79uwXL29TY76Z2rM5mHXA

spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
making mapping of every page separately. However, if the buffer is not
aligned to page boundary (e.g. sub-array in a vmalloc-ed array), it
fills the scatter table with page-size unaligned pieces, that cross
page boundaries. This is incorrect and can, for example, cause memory
corruption and various crashes when working with ubifs on spi-nor chips.

Fix this by using proper scatter table size and intra-page buffer lengths,
so that the whole buffer splits into separate scatter table entries on
page boundaries.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50910d8..14016dc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -477,7 +477,9 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 {
 	const bool vmalloced_buf = is_vmalloc_addr(buf);
 	const int desc_len = vmalloced_buf ? PAGE_SIZE : master->max_dma_len;
-	const int sgs = DIV_ROUND_UP(len, desc_len);
+	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
+					len + offset_in_page(buf) : len,
+				     desc_len);
 	struct page *vm_page;
 	void *sg_buf;
 	size_t min;
@@ -488,9 +490,10 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 		return ret;
 
 	for (i = 0; i < sgs; i++) {
-		min = min_t(size_t, len, desc_len);
 
 		if (vmalloced_buf) {
+			min = min_t(size_t,
+				    len, desc_len - offset_in_page(buf));
 			vm_page = vmalloc_to_page(buf);
 			if (!vm_page) {
 				sg_free_table(sgt);
@@ -499,6 +502,7 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 			sg_set_page(&sgt->sgl[i], vm_page,
 				    min, offset_in_page(buf));
 		} else {
+			min = min_t(size_t, len, desc_len);
 			sg_buf = buf;
 			sg_set_buf(&sgt->sgl[i], sg_buf, min);
 		}
-- 
2.1.0

--
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] 13+ messages in thread

* RE: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
       [not found] ` <1434351560-29463-1-git-send-email-andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2015-06-25 11:27   ` Andrew Gabbasov
  2015-06-25 14:00     ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-25 11:27 UTC (permalink / raw)
  To: linux-spi-u79uwXL29TY76Z2rM5mHXA; +Cc: 'Mark Brown'

> -----Original Message-----
> From: Gabbasov, Andrew
> Sent: Monday, June 15, 2015 9:59 AM
> To: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
> 
> spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
making
> mapping of every page separately. However, if the buffer is not aligned to
page
> boundary (e.g. sub-array in a vmalloc-ed array), it fills the scatter
table with
> page-size unaligned pieces, that cross page boundaries. This is incorrect
and
> can, for example, cause memory corruption and various crashes when working
> with ubifs on spi-nor chips.
> 
> Fix this by using proper scatter table size and intra-page buffer lengths,
so that
> the whole buffer splits into separate scatter table entries on page
boundaries.
> 
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/spi/spi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 50910d8..14016dc
100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -477,7 +477,9 @@ static int spi_map_buf(struct spi_master *master,
struct
> device *dev,  {
>  	const bool vmalloced_buf = is_vmalloc_addr(buf);
>  	const int desc_len = vmalloced_buf ? PAGE_SIZE : master-
> >max_dma_len;
> -	const int sgs = DIV_ROUND_UP(len, desc_len);
> +	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
> +					len + offset_in_page(buf) : len,
> +				     desc_len);
>  	struct page *vm_page;
>  	void *sg_buf;
>  	size_t min;
> @@ -488,9 +490,10 @@ static int spi_map_buf(struct spi_master *master,
> struct device *dev,
>  		return ret;
> 
>  	for (i = 0; i < sgs; i++) {
> -		min = min_t(size_t, len, desc_len);
> 
>  		if (vmalloced_buf) {
> +			min = min_t(size_t,
> +				    len, desc_len - offset_in_page(buf));
>  			vm_page = vmalloc_to_page(buf);
>  			if (!vm_page) {
>  				sg_free_table(sgt);
> @@ -499,6 +502,7 @@ static int spi_map_buf(struct spi_master *master,
struct
> device *dev,
>  			sg_set_page(&sgt->sgl[i], vm_page,
>  				    min, offset_in_page(buf));
>  		} else {
> +			min = min_t(size_t, len, desc_len);
>  			sg_buf = buf;
>  			sg_set_buf(&sgt->sgl[i], sg_buf, min);
>  		}
> --
> 2.1.0

Any opinion on this fix?

Thanks.

Best regards,
Andrew


--
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] 13+ messages in thread

* Re: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
  2015-06-25 11:27   ` Andrew Gabbasov
@ 2015-06-25 14:00     ` Mark Brown
       [not found]       ` <20150625140054.GM14071-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2015-06-25 14:00 UTC (permalink / raw)
  To: Andrew Gabbasov; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 381 bytes --]

On Thu, Jun 25, 2015 at 02:27:17PM +0300, Andrew Gabbasov wrote:
> > -----Original Message-----
> > From: Gabbasov, Andrew

> Any opinion on this fix?

Please don't quote entire messages, it makes it hard to find any new
content you have added.  If you want to submit a patch for inclusion in
the kernel please follow the patch submission process in
SubmittingPatches.  

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* RE: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
       [not found]       ` <20150625140054.GM14071-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2015-06-25 15:57         ` Gabbasov, Andrew
       [not found]           ` <6C5EA58090A5ED459815C4D04C2B466FF82082B4-Codg+kbhZ4/nlEkxMdpx1dQH9K4/4qFeAL8bYrjMMd8@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Gabbasov, Andrew @ 2015-06-25 15:57 UTC (permalink / raw)
  To: Mark Brown; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

> Please don't quote entire messages, it makes it hard to find any new
> content you have added.  If you want to submit a patch for inclusion in
> the kernel please follow the patch submission process in
> SubmittingPatches.

I did. This patch was sent to linux-spi mailing list twice (June 1 and June 15),
and only after getting no response I asked the maintainer. Sorry if it's too early,
but I believe the existing bug should be fixed. I'll submit the patch once more.

Thanks.

Best regards,
Andrew
--
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] 13+ messages in thread

* Re: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
       [not found]           ` <6C5EA58090A5ED459815C4D04C2B466FF82082B4-Codg+kbhZ4/nlEkxMdpx1dQH9K4/4qFeAL8bYrjMMd8@public.gmane.org>
@ 2015-06-25 16:12             ` Mark Brown
  0 siblings, 0 replies; 13+ messages in thread
From: Mark Brown @ 2015-06-25 16:12 UTC (permalink / raw)
  To: Gabbasov, Andrew; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Thu, Jun 25, 2015 at 03:57:50PM +0000, Gabbasov, Andrew wrote:
> > Please don't quote entire messages, it makes it hard to find any new
> > content you have added.  If you want to submit a patch for inclusion in
> > the kernel please follow the patch submission process in
> > SubmittingPatches.

> I did. This patch was sent to linux-spi mailing list twice (June 1 and June 15),
> and only after getting no response I asked the maintainer. Sorry if it's too early,
> but I believe the existing bug should be fixed. I'll submit the patch once more.

SubmittingPatches says:

| 5) Select the recipients for your patch.
| ----------------------------------------
| 
| You should always copy the appropriate subsystem maintainer(s) on any patch
| to code that they maintain; look through the MAINTAINERS file and the

which you haven't done.  If you don't send patches to people they won't
see them and can't review them.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* RE: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
       [not found]           ` <20150630160237.GR11162-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2015-06-30 16:21             ` Andrew Gabbasov
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-30 16:21 UTC (permalink / raw)
  To: 'Mark Brown'; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

> -----Original Message-----
> From: Mark Brown [mailto:broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Tuesday, June 30, 2015 7:03 PM
> To: Gabbasov, Andrew
> Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed
buffer
> 
> On Tue, Jun 30, 2015 at 06:37:07PM +0300, Andrew Gabbasov wrote:
> 
> > > The caller is supposed to be providing us with aligned memory here.
> > > However it could be helpful to do this so...
> 
> > Well, actually the rest of spi code does not rely on having a transfer
> > buffer page-aligned. And I don't see any reason to make such an
> > assumption here.
> > Especially that it is not fulfilled, at least by ubifs code.
> > Anyway, this fix seems to be useful indeed.
> 
> The requirement comes more from the underlying DMA code than from SPI
> itself, SPI mostly doesn't care.

OK, then it's all the more useful to have SPI code tolerant to arbitrary
input data
(like un-aligned addresses), so that it won't introduce its own unnecessary
restrictions.

Thanks.

Best regards,
Andrew


--
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] 13+ messages in thread

* Re: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
  2015-06-30 15:37       ` Andrew Gabbasov
@ 2015-06-30 16:02         ` Mark Brown
       [not found]           ` <20150630160237.GR11162-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2015-06-30 16:02 UTC (permalink / raw)
  To: Andrew Gabbasov; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

On Tue, Jun 30, 2015 at 06:37:07PM +0300, Andrew Gabbasov wrote:

> > The caller is supposed to be providing us with aligned memory here.
> > However it could be helpful to do this so...

> Well, actually the rest of spi code does not rely on having a transfer
> buffer
> page-aligned. And I don't see any reason to make such an assumption here.
> Especially that it is not fulfilled, at least by ubifs code.
> Anyway, this fix seems to be useful indeed.

The requirement comes more from the underlying DMA code than from SPI
itself, SPI mostly doesn't care.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* RE: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
       [not found]     ` <20150626114622.GW14071-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2015-06-30 15:37       ` Andrew Gabbasov
  2015-06-30 16:02         ` Mark Brown
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-30 15:37 UTC (permalink / raw)
  To: 'Mark Brown'; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

Hi Mark,

> -----Original Message-----
> From: Mark Brown [mailto:broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org]
> Sent: Friday, June 26, 2015 2:46 PM
> To: Gabbasov, Andrew
> Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed
buffer
> 
> On Thu, Jun 25, 2015 at 11:40:15AM -0500, Andrew Gabbasov wrote:
> > spi_map_buf() processes mapping of vmalloc-ed buffers in a special
> > way, making mapping of every page separately. However, if the buffer
> > is not aligned to page boundary (e.g. sub-array in a vmalloc-ed
> > array), it fills the scatter table with page-size unaligned pieces,
> > that cross page boundaries. This is incorrect and can, for example,
> > cause memory corruption and various crashes when working with ubifs on
spi-
> nor chips.
> 
> The caller is supposed to be providing us with aligned memory here.
> However it could be helpful to do this so...

Well, actually the rest of spi code does not rely on having a transfer
buffer
page-aligned. And I don't see any reason to make such an assumption here.
Especially that it is not fulfilled, at least by ubifs code.
Anyway, this fix seems to be useful indeed.

> > -	const int sgs = DIV_ROUND_UP(len, desc_len);
> > +	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
> > +					len + offset_in_page(buf) : len,
> > +				     desc_len);
> 
> No, please write this legibly without the ternery operator.

OK, I'll try to make this piece of code more distinct.
I'm submitting v2 of the patch.

Thanks.

Best regards,
Andrew


--
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] 13+ messages in thread

* Re: [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
       [not found] ` <1435250415-9147-1-git-send-email-andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
@ 2015-06-26 11:46   ` Mark Brown
       [not found]     ` <20150626114622.GW14071-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Mark Brown @ 2015-06-26 11:46 UTC (permalink / raw)
  To: Andrew Gabbasov; +Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA

[-- Attachment #1: Type: text/plain, Size: 837 bytes --]

On Thu, Jun 25, 2015 at 11:40:15AM -0500, Andrew Gabbasov wrote:
> spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
> making mapping of every page separately. However, if the buffer is not
> aligned to page boundary (e.g. sub-array in a vmalloc-ed array), it
> fills the scatter table with page-size unaligned pieces, that cross
> page boundaries. This is incorrect and can, for example, cause memory
> corruption and various crashes when working with ubifs on spi-nor chips.

The caller is supposed to be providing us with aligned memory here.
However it could be helpful to do this so...

> -	const int sgs = DIV_ROUND_UP(len, desc_len);
> +	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
> +					len + offset_in_page(buf) : len,
> +				     desc_len);

No, please write this legibly without the ternery operator.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
@ 2015-06-25 16:40 Andrew Gabbasov
       [not found] ` <1435250415-9147-1-git-send-email-andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-25 16:40 UTC (permalink / raw)
  To: linux-spi-u79uwXL29TY76Z2rM5mHXA; +Cc: Mark Brown

spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
making mapping of every page separately. However, if the buffer is not
aligned to page boundary (e.g. sub-array in a vmalloc-ed array), it
fills the scatter table with page-size unaligned pieces, that cross
page boundaries. This is incorrect and can, for example, cause memory
corruption and various crashes when working with ubifs on spi-nor chips.

Fix this by using proper scatter table size and intra-page buffer lengths,
so that the whole buffer splits into separate scatter table entries on
page boundaries.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50910d8..14016dc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -477,7 +477,9 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 {
 	const bool vmalloced_buf = is_vmalloc_addr(buf);
 	const int desc_len = vmalloced_buf ? PAGE_SIZE : master->max_dma_len;
-	const int sgs = DIV_ROUND_UP(len, desc_len);
+	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
+					len + offset_in_page(buf) : len,
+				     desc_len);
 	struct page *vm_page;
 	void *sg_buf;
 	size_t min;
@@ -488,9 +490,10 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 		return ret;
 
 	for (i = 0; i < sgs; i++) {
-		min = min_t(size_t, len, desc_len);
 
 		if (vmalloced_buf) {
+			min = min_t(size_t,
+				    len, desc_len - offset_in_page(buf));
 			vm_page = vmalloc_to_page(buf);
 			if (!vm_page) {
 				sg_free_table(sgt);
@@ -499,6 +502,7 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 			sg_set_page(&sgt->sgl[i], vm_page,
 				    min, offset_in_page(buf));
 		} else {
+			min = min_t(size_t, len, desc_len);
 			sg_buf = buf;
 			sg_set_buf(&sgt->sgl[i], sg_buf, min);
 		}
-- 
2.1.0

--
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] 13+ messages in thread

* [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
@ 2015-06-25 16:08 ` Andrew Gabbasov
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-25 16:08 UTC (permalink / raw)
  To: linux-spi; +Cc: linux-kernel

spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
making mapping of every page separately. However, if the buffer is not
aligned to page boundary (e.g. sub-array in a vmalloc-ed array), it
fills the scatter table with page-size unaligned pieces, that cross
page boundaries. This is incorrect and can, for example, cause memory
corruption and various crashes when working with ubifs on spi-nor chips.

Fix this by using proper scatter table size and intra-page buffer lengths,
so that the whole buffer splits into separate scatter table entries on
page boundaries.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
---
 drivers/spi/spi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50910d8..14016dc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -477,7 +477,9 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 {
 	const bool vmalloced_buf = is_vmalloc_addr(buf);
 	const int desc_len = vmalloced_buf ? PAGE_SIZE : master->max_dma_len;
-	const int sgs = DIV_ROUND_UP(len, desc_len);
+	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
+					len + offset_in_page(buf) : len,
+				     desc_len);
 	struct page *vm_page;
 	void *sg_buf;
 	size_t min;
@@ -488,9 +490,10 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 		return ret;
 
 	for (i = 0; i < sgs; i++) {
-		min = min_t(size_t, len, desc_len);
 
 		if (vmalloced_buf) {
+			min = min_t(size_t,
+				    len, desc_len - offset_in_page(buf));
 			vm_page = vmalloc_to_page(buf);
 			if (!vm_page) {
 				sg_free_table(sgt);
@@ -499,6 +502,7 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 			sg_set_page(&sgt->sgl[i], vm_page,
 				    min, offset_in_page(buf));
 		} else {
+			min = min_t(size_t, len, desc_len);
 			sg_buf = buf;
 			sg_set_buf(&sgt->sgl[i], sg_buf, min);
 		}
-- 
2.1.0


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

* [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
@ 2015-06-25 16:08 ` Andrew Gabbasov
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-25 16:08 UTC (permalink / raw)
  To: linux-spi-u79uwXL29TY76Z2rM5mHXA; +Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA

spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
making mapping of every page separately. However, if the buffer is not
aligned to page boundary (e.g. sub-array in a vmalloc-ed array), it
fills the scatter table with page-size unaligned pieces, that cross
page boundaries. This is incorrect and can, for example, cause memory
corruption and various crashes when working with ubifs on spi-nor chips.

Fix this by using proper scatter table size and intra-page buffer lengths,
so that the whole buffer splits into separate scatter table entries on
page boundaries.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50910d8..14016dc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -477,7 +477,9 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 {
 	const bool vmalloced_buf = is_vmalloc_addr(buf);
 	const int desc_len = vmalloced_buf ? PAGE_SIZE : master->max_dma_len;
-	const int sgs = DIV_ROUND_UP(len, desc_len);
+	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
+					len + offset_in_page(buf) : len,
+				     desc_len);
 	struct page *vm_page;
 	void *sg_buf;
 	size_t min;
@@ -488,9 +490,10 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 		return ret;
 
 	for (i = 0; i < sgs; i++) {
-		min = min_t(size_t, len, desc_len);
 
 		if (vmalloced_buf) {
+			min = min_t(size_t,
+				    len, desc_len - offset_in_page(buf));
 			vm_page = vmalloc_to_page(buf);
 			if (!vm_page) {
 				sg_free_table(sgt);
@@ -499,6 +502,7 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 			sg_set_page(&sgt->sgl[i], vm_page,
 				    min, offset_in_page(buf));
 		} else {
+			min = min_t(size_t, len, desc_len);
 			sg_buf = buf;
 			sg_set_buf(&sgt->sgl[i], sg_buf, min);
 		}
-- 
2.1.0

--
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] 13+ messages in thread

* [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer
@ 2015-06-01 10:14 Andrew Gabbasov
  0 siblings, 0 replies; 13+ messages in thread
From: Andrew Gabbasov @ 2015-06-01 10:14 UTC (permalink / raw)
  To: linux-spi-u79uwXL29TY76Z2rM5mHXA

spi_map_buf() processes mapping of vmalloc-ed buffers in a special way,
making mapping of every page separately. However, if the buffer is not
aligned to page boundary (e.g. sub-array in a vmalloc-ed array), it
fills the scatter table with page-size unaligned pieces, that cross
page boundaries. This is incorrect and can, for example, cause memory
corruption and various crashes when working with ubifs on spi-nor chips.

Fix this by using proper scatter table size and intra-page buffer lengths,
so that the whole buffer splits into separate scatter table entries on
page boundaries.

Signed-off-by: Andrew Gabbasov <andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50910d8..14016dc 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -477,7 +477,9 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 {
 	const bool vmalloced_buf = is_vmalloc_addr(buf);
 	const int desc_len = vmalloced_buf ? PAGE_SIZE : master->max_dma_len;
-	const int sgs = DIV_ROUND_UP(len, desc_len);
+	const int sgs = DIV_ROUND_UP(vmalloced_buf ?
+					len + offset_in_page(buf) : len,
+				     desc_len);
 	struct page *vm_page;
 	void *sg_buf;
 	size_t min;
@@ -488,9 +490,10 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 		return ret;
 
 	for (i = 0; i < sgs; i++) {
-		min = min_t(size_t, len, desc_len);
 
 		if (vmalloced_buf) {
+			min = min_t(size_t,
+				    len, desc_len - offset_in_page(buf));
 			vm_page = vmalloc_to_page(buf);
 			if (!vm_page) {
 				sg_free_table(sgt);
@@ -499,6 +502,7 @@ static int spi_map_buf(struct spi_master *master, struct device *dev,
 			sg_set_page(&sgt->sgl[i], vm_page,
 				    min, offset_in_page(buf));
 		} else {
+			min = min_t(size_t, len, desc_len);
 			sg_buf = buf;
 			sg_set_buf(&sgt->sgl[i], sg_buf, min);
 		}
-- 
2.1.0

--
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] 13+ messages in thread

end of thread, other threads:[~2015-06-30 16:21 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-15  6:59 [PATCH] spi: Fix per-page mapping of unaligned vmalloc-ed buffer Andrew Gabbasov
     [not found] ` <1434351560-29463-1-git-send-email-andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2015-06-25 11:27   ` Andrew Gabbasov
2015-06-25 14:00     ` Mark Brown
     [not found]       ` <20150625140054.GM14071-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-06-25 15:57         ` Gabbasov, Andrew
     [not found]           ` <6C5EA58090A5ED459815C4D04C2B466FF82082B4-Codg+kbhZ4/nlEkxMdpx1dQH9K4/4qFeAL8bYrjMMd8@public.gmane.org>
2015-06-25 16:12             ` Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2015-06-25 16:40 Andrew Gabbasov
     [not found] ` <1435250415-9147-1-git-send-email-andrew_gabbasov-nmGgyN9QBj3QT0dZR+AlfA@public.gmane.org>
2015-06-26 11:46   ` Mark Brown
     [not found]     ` <20150626114622.GW14071-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-06-30 15:37       ` Andrew Gabbasov
2015-06-30 16:02         ` Mark Brown
     [not found]           ` <20150630160237.GR11162-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2015-06-30 16:21             ` Andrew Gabbasov
2015-06-25 16:08 Andrew Gabbasov
2015-06-25 16:08 ` Andrew Gabbasov
2015-06-01 10:14 Andrew Gabbasov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.