[PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[David Cohen]

The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
quirk implemented to align buffer size to maxpacketsize on out endpoint.
As result, functionfs does not work on Intel platforms using dwc3 driver
(i.e. Bay Trail and Merrifield). This patch fixes the issue.

This code is based on a previous Qiuxu's patch.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
---

Hi,

Since this is a feature that worked in past, this patch is meant for 3.17
kernel. If/when we pass review and accept it on 3.17, I'll send a version for
stable 3.16 kernel too. It is not required for 3.14, since the regression came
later.

Br, David Cohen

---
 drivers/usb/gadget/function/f_fs.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 0dc3552d1360..6e2b8063b170 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -648,15 +648,26 @@ static void ffs_user_copy_worker(struct work_struct *work)
 	if (io_data->read && ret > 0) {
 		int i;
 		size_t pos = 0;
+
+		/*
+		 * Since req->length may be bigger than io_data->len (after
+		 * being rounded up to maxpacketsize), we may end up with more
+		 * data then user space has space for.
+		 */
+		ret = min_t(int, ret, io_data->len);
+
 		use_mm(io_data->mm);
 		for (i = 0; i < io_data->nr_segs; i++) {
+			size_t len = min_t(size_t, ret - pos,
+					io_data->iovec[i].iov_len);
+			if (!len)
+				break;
 			if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
-						 &io_data->buf[pos],
-						 io_data->iovec[i].iov_len))) {
+						 &io_data->buf[pos], len))) {
 				ret = -EFAULT;
 				break;
 			}
-			pos += io_data->iovec[i].iov_len;
+			pos += len;
 		}
 		unuse_mm(io_data->mm);
 	}
@@ -794,7 +805,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 				goto error_lock;
 
 			req->buf      = data;
-			req->length   = io_data->len;
+			req->length   = data_len;
 
 			io_data->buf = data;
 			io_data->ep = ep->ep;
@@ -816,7 +827,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 
 			req = ep->req;
 			req->buf      = data;
-			req->length   = io_data->len;
+			req->length   = data_len;
 
 			req->context  = &done;
 			req->complete = ffs_epfile_io_complete;
-- 
2.1.0

Re: [PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Felipe Balbi]

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

Hi,

On Tue, Oct 07, 2014 at 05:18:06PM -0700, David Cohen wrote:
> The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the

did that commit non-aio or is only aio broken ?

> quirk implemented to align buffer size to maxpacketsize on out endpoint.
> As result, functionfs does not work on Intel platforms using dwc3 driver
> (i.e. Bay Trail and Merrifield). This patch fixes the issue.

and Braswell.

> This code is based on a previous Qiuxu's patch.

How have you tested this ? Do you have a test-case to share ? Then I can
add it to my tests which I have been running on my platforms too (they
include USB20CV and USB30CV where applicable - quite a few fixes coming
soon).

cheers

-- 
balbi

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

Re: [PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[David Cohen]

On Tue, Oct 07, 2014 at 07:31:44PM -0500, Felipe Balbi wrote:
> Hi,

Hi Felipe,

> 
> On Tue, Oct 07, 2014 at 05:18:06PM -0700, David Cohen wrote:
> > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> 
> did that commit non-aio or is only aio broken ?

That commit broke the quirk on both cases.

> 
> > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > As result, functionfs does not work on Intel platforms using dwc3 driver
> > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> 
> and Braswell.
> 
> > This code is based on a previous Qiuxu's patch.
> 
> How have you tested this ? Do you have a test-case to share ? Then I can
> add it to my tests which I have been running on my platforms too (they
> include USB20CV and USB30CV where applicable - quite a few fixes coming
> soon).

My testcase is to use Android's adbd service via function_fs (no
out-of-tree android gadget). Without the quitk, it doesn't work.

Br, David

> 
> cheers
> 
> -- 
> balbi

Re: [PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Felipe Balbi]

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

Hi again,

On Tue, Oct 07, 2014 at 05:18:06PM -0700, David Cohen wrote:
> The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> quirk implemented to align buffer size to maxpacketsize on out endpoint.
> As result, functionfs does not work on Intel platforms using dwc3 driver
> (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> 
> This code is based on a previous Qiuxu's patch.
> 

btw, please resend and add below right here:

Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
Cc: <stable@vger.kernel.org> # v3.16+

> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>

-- 
balbi

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

Re: [PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[David Cohen]

On Tue, Oct 07, 2014 at 07:32:56PM -0500, Felipe Balbi wrote:
> Hi again,
> 
> On Tue, Oct 07, 2014 at 05:18:06PM -0700, David Cohen wrote:
> > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > As result, functionfs does not work on Intel platforms using dwc3 driver
> > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> > 
> > This code is based on a previous Qiuxu's patch.
> > 
> 
> btw, please resend and add below right here:
> 
> Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> Cc: <stable@vger.kernel.org> # v3.16+

Sure. But v3.16 doesn't have the new 'function' directory under usb/gadget.
Not sure if same patch is useful for v3.16+.
The patch I sent is intended for v3.17.1 and v3.18-rc1.

Br, David

> 
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> 
> -- 
> balbi

Re: [PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Felipe Balbi]

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

Hi,

On Wed, Oct 08, 2014 at 10:55:58AM -0700, David Cohen wrote:
> On Tue, Oct 07, 2014 at 07:32:56PM -0500, Felipe Balbi wrote:
> > Hi again,
> > 
> > On Tue, Oct 07, 2014 at 05:18:06PM -0700, David Cohen wrote:
> > > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> > > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > > As result, functionfs does not work on Intel platforms using dwc3 driver
> > > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> > > 
> > > This code is based on a previous Qiuxu's patch.
> > > 
> > 
> > btw, please resend and add below right here:
> > 
> > Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> > Cc: <stable@vger.kernel.org> # v3.16+
> 
> Sure. But v3.16 doesn't have the new 'function' directory under usb/gadget.
> Not sure if same patch is useful for v3.16+.
> The patch I sent is intended for v3.17.1 and v3.18-rc1.

code is the same anyway right ? It's just a path change :-)

-- 
balbi

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

Re: [PATCH] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Michal Nazarewicz]

On Tue, Oct 07 2014, David Cohen <david.a.cohen@linux.intel.com> wrote:
> The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> quirk implemented to align buffer size to maxpacketsize on out endpoint.
> As result, functionfs does not work on Intel platforms using dwc3 driver
> (i.e. Bay Trail and Merrifield). This patch fixes the issue.
>
> This code is based on a previous Qiuxu's patch.
>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>

Acked-by: Michal Nazarewicz <mina86@mina86.com>

> ---
>
> Hi,
>
> Since this is a feature that worked in past, this patch is meant for 3.17
> kernel. If/when we pass review and accept it on 3.17, I'll send a version for
> stable 3.16 kernel too. It is not required for 3.14, since the regression came
> later.
>
> Br, David Cohen
>
> ---
>  drivers/usb/gadget/function/f_fs.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
> index 0dc3552d1360..6e2b8063b170 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -648,15 +648,26 @@ static void ffs_user_copy_worker(struct work_struct *work)
>  	if (io_data->read && ret > 0) {
>  		int i;
>  		size_t pos = 0;
> +
> +		/*
> +		 * Since req->length may be bigger than io_data->len (after
> +		 * being rounded up to maxpacketsize), we may end up with more
> +		 * data then user space has space for.
> +		 */
> +		ret = min_t(int, ret, io_data->len);
> +
>  		use_mm(io_data->mm);
>  		for (i = 0; i < io_data->nr_segs; i++) {
> +			size_t len = min_t(size_t, ret - pos,
> +					io_data->iovec[i].iov_len);
> +			if (!len)
> +				break;
>  			if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
> -						 &io_data->buf[pos],
> -						 io_data->iovec[i].iov_len))) {
> +						 &io_data->buf[pos], len))) {
>  				ret = -EFAULT;
>  				break;
>  			}
> -			pos += io_data->iovec[i].iov_len;
> +			pos += len;
>  		}
>  		unuse_mm(io_data->mm);
>  	}
> @@ -794,7 +805,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
>  				goto error_lock;
>  
>  			req->buf      = data;
> -			req->length   = io_data->len;
> +			req->length   = data_len;
>  
>  			io_data->buf = data;
>  			io_data->ep = ep->ep;
> @@ -816,7 +827,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
>  
>  			req = ep->req;
>  			req->buf      = data;
> -			req->length   = io_data->len;
> +			req->length   = data_len;
>  
>  			req->context  = &done;
>  			req->complete = ffs_epfile_io_complete;
> -- 
> 2.1.0
>

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--

[PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[David Cohen]

The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
quirk implemented to align buffer size to maxpacketsize on out endpoint.
As result, functionfs does not work on Intel platforms using dwc3 driver
(i.e. Bay Trail and Merrifield). This patch fixes the issue.

This code is based on a previous Qiuxu's patch.

Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
Cc: <stable@vger.kernel.org> # v3.16+
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---

Hi,

Since this is a feature that worked in past, this is meant for stable
versions >= 3.16 too.

v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.

Br, David Cohen

---
 drivers/usb/gadget/function/f_fs.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 0dc3552d1360..6e2b8063b170 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -648,15 +648,26 @@ static void ffs_user_copy_worker(struct work_struct *work)
 	if (io_data->read && ret > 0) {
 		int i;
 		size_t pos = 0;
+
+		/*
+		 * Since req->length may be bigger than io_data->len (after
+		 * being rounded up to maxpacketsize), we may end up with more
+		 * data then user space has space for.
+		 */
+		ret = min_t(int, ret, io_data->len);
+
 		use_mm(io_data->mm);
 		for (i = 0; i < io_data->nr_segs; i++) {
+			size_t len = min_t(size_t, ret - pos,
+					io_data->iovec[i].iov_len);
+			if (!len)
+				break;
 			if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
-						 &io_data->buf[pos],
-						 io_data->iovec[i].iov_len))) {
+						 &io_data->buf[pos], len))) {
 				ret = -EFAULT;
 				break;
 			}
-			pos += io_data->iovec[i].iov_len;
+			pos += len;
 		}
 		unuse_mm(io_data->mm);
 	}
@@ -794,7 +805,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 				goto error_lock;
 
 			req->buf      = data;
-			req->length   = io_data->len;
+			req->length   = data_len;
 
 			io_data->buf = data;
 			io_data->ep = ep->ep;
@@ -816,7 +827,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 
 			req = ep->req;
 			req->buf      = data;
-			req->length   = io_data->len;
+			req->length   = data_len;
 
 			req->context  = &done;
 			req->complete = ffs_epfile_io_complete;
-- 
2.1.0

Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Al Viro]

On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
>  		use_mm(io_data->mm);
>  		for (i = 0; i < io_data->nr_segs; i++) {
> +			size_t len = min_t(size_t, ret - pos,
> +					io_data->iovec[i].iov_len);
> +			if (!len)
> +				break;
>  			if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
> -						 &io_data->buf[pos],
> -						 io_data->iovec[i].iov_len))) {
> +						 &io_data->buf[pos], len))) {
>  				ret = -EFAULT;
>  				break;
>  			}
> -			pos += io_data->iovec[i].iov_len;
> +			pos += len;

Hmm...  This is really asking for something like
	if (copy_to_iter(io_data->buf, ret, <something>) != ret)
		ret = -EFAULT;
with <something> being an iov_iter instead of iovec.  It would be really
nice to have that thing switched to ->read_iter/->write_iter, dropping
->read/->write/->aio_read/->aio_write; I'm not familiar enough with that
code to do it on my own, though, so it would require some help from
maintainers...

Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Felipe Balbi]

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

On Sun, Oct 12, 2014 at 08:12:28PM +0100, Al Viro wrote:
> On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> >  		use_mm(io_data->mm);
> >  		for (i = 0; i < io_data->nr_segs; i++) {
> > +			size_t len = min_t(size_t, ret - pos,
> > +					io_data->iovec[i].iov_len);
> > +			if (!len)
> > +				break;
> >  			if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
> > -						 &io_data->buf[pos],
> > -						 io_data->iovec[i].iov_len))) {
> > +						 &io_data->buf[pos], len))) {
> >  				ret = -EFAULT;
> >  				break;
> >  			}
> > -			pos += io_data->iovec[i].iov_len;
> > +			pos += len;
> 
> Hmm...  This is really asking for something like
> 	if (copy_to_iter(io_data->buf, ret, <something>) != ret)
> 		ret = -EFAULT;
> with <something> being an iov_iter instead of iovec.  It would be really
> nice to have that thing switched to ->read_iter/->write_iter, dropping
> ->read/->write/->aio_read/->aio_write; I'm not familiar enough with that
> code to do it on my own, though, so it would require some help from
> maintainers...

cool, Michal, if you're too busy lately, I can look at that one myself.
I suppose the test application we have under tool/usb is good enough for
validation ?

cheers

-- 
balbi

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

Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Felipe Balbi]

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

On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> quirk implemented to align buffer size to maxpacketsize on out endpoint.
> As result, functionfs does not work on Intel platforms using dwc3 driver
> (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> 
> This code is based on a previous Qiuxu's patch.
> 
> Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> Cc: <stable@vger.kernel.org> # v3.16+
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> ---
> 
> Hi,
> 
> Since this is a feature that worked in past, this is meant for stable
> versions >= 3.16 too.
> 
> v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.

this adds a build warning for use of maybe unitialized data_len. Plese
fix.

-- 
balbi

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

Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[David Cohen]

On Mon, Oct 13, 2014 at 10:32:12AM -0500, Felipe Balbi wrote:
> On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > As result, functionfs does not work on Intel platforms using dwc3 driver
> > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> > 
> > This code is based on a previous Qiuxu's patch.
> > 
> > Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> > Cc: <stable@vger.kernel.org> # v3.16+
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> > ---
> > 
> > Hi,
> > 
> > Since this is a feature that worked in past, this is meant for stable
> > versions >= 3.16 too.
> > 
> > v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.
> 
> this adds a build warning for use of maybe unitialized data_len. Plese
> fix.

It's a false-positive warning. data_len is only initialized if halt != 0
and it's only used if halt != 0 too.

Do you prefer to initialize it to 0 during the declaration to silent the
compiler?

BR, David

> 
> -- 
> balbi

Re: [PATCH v2] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[Felipe Balbi]

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

On Mon, Oct 13, 2014 at 09:55:56AM -0700, David Cohen wrote:
> On Mon, Oct 13, 2014 at 10:32:12AM -0500, Felipe Balbi wrote:
> > On Wed, Oct 08, 2014 at 02:12:18PM -0700, David Cohen wrote:
> > > The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
> > > quirk implemented to align buffer size to maxpacketsize on out endpoint.
> > > As result, functionfs does not work on Intel platforms using dwc3 driver
> > > (i.e. Bay Trail and Merrifield). This patch fixes the issue.
> > > 
> > > This code is based on a previous Qiuxu's patch.
> > > 
> > > Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
> > > Cc: <stable@vger.kernel.org> # v3.16+
> > > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > > Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
> > > Acked-by: Michal Nazarewicz <mina86@mina86.com>
> > > ---
> > > 
> > > Hi,
> > > 
> > > Since this is a feature that worked in past, this is meant for stable
> > > versions >= 3.16 too.
> > > 
> > > v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.
> > 
> > this adds a build warning for use of maybe unitialized data_len. Plese
> > fix.
> 
> It's a false-positive warning. data_len is only initialized if halt != 0
> and it's only used if halt != 0 too.
> 
> Do you prefer to initialize it to 0 during the declaration to silent the
> compiler?

yup.

-- 
balbi

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

[PATCH v3] usb: ffs: fix regression when quirk_ep_out_aligned_size flag is set

[David Cohen]

The commit '2e4c7553cd usb: gadget: f_fs: add aio support' broke the
quirk implemented to align buffer size to maxpacketsize on out endpoint.
As result, functionfs does not work on Intel platforms using dwc3 driver
(i.e. Bay Trail and Merrifield). This patch fixes the issue.

This code is based on a previous Qiuxu's patch.

Fixes: 2e4c7553cd (usb: gadget: f_fs: add aio support)
Cc: <stable@vger.kernel.org> # v3.16+
Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@intel.com>
Acked-by: Michal Nazarewicz <mina86@mina86.com>
---

Hi,

Since this is a feature that worked in past, this is meant for stable
versions >= 3.16 too.

v1 to v2: just added Fixes, Cc and Acked-by lines on patch description.
v2 to v3: fixed compiler warning about data_len being used unitialized

Br, David Cohen

---
 drivers/usb/gadget/function/f_fs.c | 40 ++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 0dc3552d1360..9b6bc4d30352 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -648,15 +648,26 @@ static void ffs_user_copy_worker(struct work_struct *work)
 	if (io_data->read && ret > 0) {
 		int i;
 		size_t pos = 0;
+
+		/*
+		 * Since req->length may be bigger than io_data->len (after
+		 * being rounded up to maxpacketsize), we may end up with more
+		 * data then user space has space for.
+		 */
+		ret = min_t(int, ret, io_data->len);
+
 		use_mm(io_data->mm);
 		for (i = 0; i < io_data->nr_segs; i++) {
+			size_t len = min_t(size_t, ret - pos,
+					io_data->iovec[i].iov_len);
+			if (!len)
+				break;
 			if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
-						 &io_data->buf[pos],
-						 io_data->iovec[i].iov_len))) {
+						 &io_data->buf[pos], len))) {
 				ret = -EFAULT;
 				break;
 			}
-			pos += io_data->iovec[i].iov_len;
+			pos += len;
 		}
 		unuse_mm(io_data->mm);
 	}
@@ -688,7 +699,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 	struct ffs_epfile *epfile = file->private_data;
 	struct ffs_ep *ep;
 	char *data = NULL;
-	ssize_t ret, data_len;
+	ssize_t ret, data_len = -EINVAL;
 	int halt;
 
 	/* Are we still active? */
@@ -788,13 +799,30 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 		/* Fire the request */
 		struct usb_request *req;
 
+		/*
+		 * Sanity Check: even though data_len can't be used
+		 * uninitialized at the time I write this comment, some
+		 * compilers complain about this situation.
+		 * In order to keep the code clean from warnings, data_len is
+		 * being initialized to -EINVAL during its declaration, which
+		 * means we can't rely on compiler anymore to warn no future
+		 * changes won't result in data_len being used uninitialized.
+		 * For such reason, we're adding this redundant sanity check
+		 * here.
+		 */
+		if (unlikely(data_len == -EINVAL)) {
+			WARN(1, "%s: data_len == -EINVAL\n", __func__);
+			ret = -EINVAL;
+			goto error_lock;
+		}
+
 		if (io_data->aio) {
 			req = usb_ep_alloc_request(ep->ep, GFP_KERNEL);
 			if (unlikely(!req))
 				goto error_lock;
 
 			req->buf      = data;
-			req->length   = io_data->len;
+			req->length   = data_len;
 
 			io_data->buf = data;
 			io_data->ep = ep->ep;
@@ -816,7 +844,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
 
 			req = ep->req;
 			req->buf      = data;
-			req->length   = io_data->len;
+			req->length   = data_len;
 
 			req->context  = &done;
 			req->complete = ffs_epfile_io_complete;
-- 
2.1.0