Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
On Tue, Mar 04, 2014 at 11:38:32PM +, Liu, Chuansheng wrote: > Hi Balbi, > > > -Original Message- > > From: Felipe Balbi [mailto:ba...@ti.com] > > Sent: Wednesday, March 05, 2014 3:56 AM > > To: Michal Nazarewicz > > Cc: Robert Baldyga; Felipe Balbi; Sergei Shtylyov; Liu, Chuansheng; > > gre...@linuxfoundation.org; linux-...@vger.kernel.org; > > linux-kernel@vger.kernel.org; david.a.co...@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 signature.asc Description: Digital signature
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
On Tue, Mar 04, 2014 at 11:38:32PM +, Liu, Chuansheng wrote: Hi Balbi, -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Wednesday, March 05, 2014 3:56 AM To: Michal Nazarewicz Cc: Robert Baldyga; Felipe Balbi; Sergei Shtylyov; Liu, Chuansheng; gre...@linuxfoundation.org; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; david.a.co...@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 signature.asc Description: Digital signature
RE: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
Hi Balbi, > -Original Message- > From: Felipe Balbi [mailto:ba...@ti.com] > Sent: Wednesday, March 05, 2014 3:56 AM > To: Michal Nazarewicz > Cc: Robert Baldyga; Felipe Balbi; Sergei Shtylyov; Liu, Chuansheng; > gre...@linuxfoundation.org; linux-...@vger.kernel.org; > linux-kernel@vger.kernel.org; david.a.co...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
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. -- balbi signature.asc Description: Digital signature
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
>> 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 +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
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 > >Reviewed-by: David Cohen > >Signed-off-by: Chuansheng Liu > >--- > > 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 signature.asc Description: Digital signature
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
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
Reviewed-by: David Cohen
Signed-off-by: Chuansheng Liu
---
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
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 min...@mina86.com
Reviewed-by: David Cohen david.a.co...@linux.intel.com
Signed-off-by: Chuansheng Liu chuansheng@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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
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 min...@mina86.com Reviewed-by: David Cohen david.a.co...@linux.intel.com Signed-off-by: Chuansheng Liu chuansheng@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 signature.asc Description: Digital signature
Re: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
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 +--m...@google.com--xmpp:min...@jabber.org--ooO--(_)--Ooo-- signature.asc Description: PGP signature
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. -- balbi signature.asc Description: Digital signature
RE: [PATCH v2] usb: gadget: return the right length in ffs_epfile_io()
Hi Balbi, -Original Message- From: Felipe Balbi [mailto:ba...@ti.com] Sent: Wednesday, March 05, 2014 3:56 AM To: Michal Nazarewicz Cc: Robert Baldyga; Felipe Balbi; Sergei Shtylyov; Liu, Chuansheng; gre...@linuxfoundation.org; linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; david.a.co...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
