* [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
@ 2014-03-04 7:34 Chuansheng Liu
2014-03-04 17:01 ` Sergei Shtylyov
0 siblings, 1 reply; 7+ messages in thread
From: Chuansheng Liu @ 2014-03-04 7:34 UTC (permalink / raw)
To: balbi, gregkh
Cc: linux-usb, linux-kernel, mina86, david.a.cohen, Chuansheng Liu
When the request length is aligned to maxpacketsize, sometimes
the return length ret > the user space requested len.
At that time, we will use min_t(size_t, ret, len) to limit the
size in case of user data buffer overflow.
But we need return the min_t(size_t, ret, len) to tell the user
space rightly also.
Acked-by: Michal Nazarewicz <mina86@mina86.com>
Reviewed-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
---
drivers/usb/gadget/f_fs.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
index 42f7a0e..780f877 100644
--- a/drivers/usb/gadget/f_fs.c
+++ b/drivers/usb/gadget/f_fs.c
@@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
* we may end up with more data then user space has
* space for.
*/
- ret = ep->status;
- if (io_data->read && ret > 0 &&
- unlikely(copy_to_user(io_data->buf, data,
- min_t(size_t, ret,
- io_data->len))))
- ret = -EFAULT;
+ ret = ep->status;
+ if (io_data->read && ret > 0) {
+ ret = min_t(size_t, ret, io_data->len);
+
+ if (unlikely(copy_to_user(io_data->buf,
+ data, ret)))
+ ret = -EFAULT;
+ }
}
kfree(data);
}
--
1.9.rc0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
2014-03-04 7:34 [PATCH v2] usb: gadget: return the right length in ffs_epfile_io() Chuansheng Liu
@ 2014-03-04 17:01 ` Sergei Shtylyov
2014-03-04 16:06 ` Felipe Balbi
0 siblings, 1 reply; 7+ messages in thread
From: Sergei Shtylyov @ 2014-03-04 17:01 UTC (permalink / raw)
To: Chuansheng Liu, balbi, gregkh
Cc: linux-usb, linux-kernel, mina86, david.a.cohen
Hello.
On 03/04/2014 10:34 AM, Chuansheng Liu wrote:
> When the request length is aligned to maxpacketsize, sometimes
> the return length ret > the user space requested len.
> At that time, we will use min_t(size_t, ret, len) to limit the
> size in case of user data buffer overflow.
> But we need return the min_t(size_t, ret, len) to tell the user
> space rightly also.
> Acked-by: Michal Nazarewicz <mina86@mina86.com>
> Reviewed-by: David Cohen <david.a.cohen@linux.intel.com>
> Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> ---
> drivers/usb/gadget/f_fs.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
> diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> index 42f7a0e..780f877 100644
> --- a/drivers/usb/gadget/f_fs.c
> +++ b/drivers/usb/gadget/f_fs.c
> @@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
> * we may end up with more data then user space has
> * space for.
> */
> - ret = ep->status;
> - if (io_data->read && ret > 0 &&
> - unlikely(copy_to_user(io_data->buf, data,
> - min_t(size_t, ret,
> - io_data->len))))
> - ret = -EFAULT;
> + ret = ep->status;
Why the indentation jumped suddenly to the right?
> + if (io_data->read && ret > 0) {
> + ret = min_t(size_t, ret, io_data->len);
> +
> + if (unlikely(copy_to_user(io_data->buf,
> + data, ret)))
> + ret = -EFAULT;
> + }
> }
> kfree(data);
WBR, Sergei
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
2014-03-04 17:01 ` Sergei Shtylyov
@ 2014-03-04 16:06 ` Felipe Balbi
2014-03-04 19:53 ` Michal Nazarewicz
0 siblings, 1 reply; 7+ messages in thread
From: Felipe Balbi @ 2014-03-04 16:06 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Chuansheng Liu, balbi, gregkh, linux-usb, linux-kernel, mina86,
david.a.cohen
[-- Attachment #1: Type: text/plain, Size: 1487 bytes --]
On Tue, Mar 04, 2014 at 08:01:15PM +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 03/04/2014 10:34 AM, Chuansheng Liu wrote:
>
> >When the request length is aligned to maxpacketsize, sometimes
> >the return length ret > the user space requested len.
>
> >At that time, we will use min_t(size_t, ret, len) to limit the
> >size in case of user data buffer overflow.
>
> >But we need return the min_t(size_t, ret, len) to tell the user
> >space rightly also.
>
> >Acked-by: Michal Nazarewicz <mina86@mina86.com>
> >Reviewed-by: David Cohen <david.a.cohen@linux.intel.com>
> >Signed-off-by: Chuansheng Liu <chuansheng.liu@intel.com>
> >---
> > drivers/usb/gadget/f_fs.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
>
> >diff --git a/drivers/usb/gadget/f_fs.c b/drivers/usb/gadget/f_fs.c
> >index 42f7a0e..780f877 100644
> >--- a/drivers/usb/gadget/f_fs.c
> >+++ b/drivers/usb/gadget/f_fs.c
> >@@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
> > * we may end up with more data then user space has
> > * space for.
> > */
> >- ret = ep->status;
> >- if (io_data->read && ret > 0 &&
> >- unlikely(copy_to_user(io_data->buf, data,
> >- min_t(size_t, ret,
> >- io_data->len))))
> >- ret = -EFAULT;
> >+ ret = ep->status;
>
> Why the indentation jumped suddenly to the right?
because it was wrong before ;-)
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
2014-03-04 16:06 ` Felipe Balbi
@ 2014-03-04 19:53 ` Michal Nazarewicz
2014-03-04 19:55 ` Felipe Balbi
0 siblings, 1 reply; 7+ messages in thread
From: Michal Nazarewicz @ 2014-03-04 19:53 UTC (permalink / raw)
To: Robert Baldyga, Felipe Balbi, Sergei Shtylyov
Cc: Chuansheng Liu, balbi, gregkh, linux-usb, linux-kernel,
david.a.cohen
[-- Attachment #1: Type: text/plain, Size: 1145 bytes --]
>> On 03/04/2014 10:34 AM, Chuansheng Liu wrote:
>> >@@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
>> > * we may end up with more data then user space has
>> > * space for.
>> > */
>> >- ret = ep->status;
>> >- if (io_data->read && ret > 0 &&
>> >- unlikely(copy_to_user(io_data->buf, data,
>> >- min_t(size_t, ret,
>> >- io_data->len))))
>> >- ret = -EFAULT;
>> >+ ret = ep->status;
On Tue, Mar 04 2014, Felipe Balbi wrote:
>> Why the indentation jumped suddenly to the right?
> On Tue, Mar 04, 2014 at 08:01:15PM +0300, Sergei Shtylyov wrote:
> because it was wrong before ;-)
Yep. It looks like Robert's [2e4c7553: add aio support] introduced an
if-else-if-else flow but did not indent the code and I didn't caught it
when reviewing that patch.
--
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--
[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 835 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
2014-03-04 19:53 ` Michal Nazarewicz
@ 2014-03-04 19:55 ` Felipe Balbi
2014-03-04 23:38 ` Liu, Chuansheng
0 siblings, 1 reply; 7+ messages in thread
From: Felipe Balbi @ 2014-03-04 19:55 UTC (permalink / raw)
To: Michal Nazarewicz
Cc: Robert Baldyga, Felipe Balbi, Sergei Shtylyov, Chuansheng Liu,
gregkh, linux-usb, linux-kernel, david.a.cohen
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
On Tue, Mar 04, 2014 at 08:53:40PM +0100, Michal Nazarewicz wrote:
> >> On 03/04/2014 10:34 AM, Chuansheng Liu wrote:
> >> >@@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
> >> > * we may end up with more data then user space has
> >> > * space for.
> >> > */
> >> >- ret = ep->status;
> >> >- if (io_data->read && ret > 0 &&
> >> >- unlikely(copy_to_user(io_data->buf, data,
> >> >- min_t(size_t, ret,
> >> >- io_data->len))))
> >> >- ret = -EFAULT;
> >> >+ ret = ep->status;
>
> On Tue, Mar 04 2014, Felipe Balbi wrote:
> >> Why the indentation jumped suddenly to the right?
>
> > On Tue, Mar 04, 2014 at 08:01:15PM +0300, Sergei Shtylyov wrote:
> > because it was wrong before ;-)
>
> Yep. It looks like Robert's [2e4c7553: add aio support] introduced an
> if-else-if-else flow but did not indent the code and I didn't caught it
> when reviewing that patch.
it's in my testing/next now, I also fixed the comment indentation which
was also wrong.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
2014-03-04 19:55 ` Felipe Balbi
@ 2014-03-04 23:38 ` Liu, Chuansheng
2014-03-05 15:22 ` Felipe Balbi
0 siblings, 1 reply; 7+ messages in thread
From: Liu, Chuansheng @ 2014-03-04 23:38 UTC (permalink / raw)
To: balbi@ti.com, Michal Nazarewicz
Cc: Robert Baldyga, Sergei Shtylyov, gregkh@linuxfoundation.org,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
david.a.cohen@linux.intel.com
Hi Balbi,
> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@ti.com]
> Sent: Wednesday, March 05, 2014 3:56 AM
> To: Michal Nazarewicz
> Cc: Robert Baldyga; Felipe Balbi; Sergei Shtylyov; Liu, Chuansheng;
> gregkh@linuxfoundation.org; linux-usb@vger.kernel.org;
> linux-kernel@vger.kernel.org; david.a.cohen@linux.intel.com
> Subject: Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
>
> On Tue, Mar 04, 2014 at 08:53:40PM +0100, Michal Nazarewicz wrote:
> > >> On 03/04/2014 10:34 AM, Chuansheng Liu wrote:
> > >> >@@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file,
> struct ffs_io_data *io_data)
> > >> > * we may end up with more data then user space has
> > >> > * space for.
> > >> > */
> > >> >- ret = ep->status;
> > >> >- if (io_data->read && ret > 0 &&
> > >> >- unlikely(copy_to_user(io_data->buf, data,
> > >> >- min_t(size_t, ret,
> > >> >- io_data->len))))
> > >> >- ret = -EFAULT;
> > >> >+ ret = ep->status;
> >
> > On Tue, Mar 04 2014, Felipe Balbi wrote:
> > >> Why the indentation jumped suddenly to the right?
> >
> > > On Tue, Mar 04, 2014 at 08:01:15PM +0300, Sergei Shtylyov wrote:
> > > because it was wrong before ;-)
> >
> > Yep. It looks like Robert's [2e4c7553: add aio support] introduced an
> > if-else-if-else flow but did not indent the code and I didn't caught it
> > when reviewing that patch.
>
> it's in my testing/next now, I also fixed the comment indentation which
> was also wrong.
Thanks your help and the fix for comment indentation also:)
Best Regards
Chuansheng
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
2014-03-04 23:38 ` Liu, Chuansheng
@ 2014-03-05 15:22 ` Felipe Balbi
0 siblings, 0 replies; 7+ messages in thread
From: Felipe Balbi @ 2014-03-05 15:22 UTC (permalink / raw)
To: Liu, Chuansheng
Cc: balbi@ti.com, Michal Nazarewicz, Robert Baldyga, Sergei Shtylyov,
gregkh@linuxfoundation.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, david.a.cohen@linux.intel.com
[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]
On Tue, Mar 04, 2014 at 11:38:32PM +0000, Liu, Chuansheng wrote:
> Hi Balbi,
>
> > -----Original Message-----
> > From: Felipe Balbi [mailto:balbi@ti.com]
> > Sent: Wednesday, March 05, 2014 3:56 AM
> > To: Michal Nazarewicz
> > Cc: Robert Baldyga; Felipe Balbi; Sergei Shtylyov; Liu, Chuansheng;
> > gregkh@linuxfoundation.org; linux-usb@vger.kernel.org;
> > linux-kernel@vger.kernel.org; david.a.cohen@linux.intel.com
> > Subject: Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
> >
> > On Tue, Mar 04, 2014 at 08:53:40PM +0100, Michal Nazarewicz wrote:
> > > >> On 03/04/2014 10:34 AM, Chuansheng Liu wrote:
> > > >> >@@ -845,12 +845,14 @@ static ssize_t ffs_epfile_io(struct file *file,
> > struct ffs_io_data *io_data)
> > > >> > * we may end up with more data then user space has
> > > >> > * space for.
> > > >> > */
> > > >> >- ret = ep->status;
> > > >> >- if (io_data->read && ret > 0 &&
> > > >> >- unlikely(copy_to_user(io_data->buf, data,
> > > >> >- min_t(size_t, ret,
> > > >> >- io_data->len))))
> > > >> >- ret = -EFAULT;
> > > >> >+ ret = ep->status;
> > >
> > > On Tue, Mar 04 2014, Felipe Balbi wrote:
> > > >> Why the indentation jumped suddenly to the right?
> > >
> > > > On Tue, Mar 04, 2014 at 08:01:15PM +0300, Sergei Shtylyov wrote:
> > > > because it was wrong before ;-)
> > >
> > > Yep. It looks like Robert's [2e4c7553: add aio support] introduced an
> > > if-else-if-else flow but did not indent the code and I didn't caught it
> > > when reviewing that patch.
> >
> > it's in my testing/next now, I also fixed the comment indentation which
> > was also wrong.
> Thanks your help and the fix for comment indentation also:)
no problem, cheers ;-)
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-03-05 15:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-04 7:34 [PATCH v2] usb: gadget: return the right length in ffs_epfile_io() Chuansheng Liu
2014-03-04 17:01 ` Sergei Shtylyov
2014-03-04 16:06 ` Felipe Balbi
2014-03-04 19:53 ` Michal Nazarewicz
2014-03-04 19:55 ` Felipe Balbi
2014-03-04 23:38 ` Liu, Chuansheng
2014-03-05 15:22 ` Felipe Balbi
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.