| Message ID | 1506574207-28971-1-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Accepted |
| Commit | 72ac8f3fc29016a31ee309b4d025b487e78906ab |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [U-Boot] usb: storage: Fix overwritten in usb_stor_set_max_xfer_blk() | expand |
On 09/28/2017 06:50 AM, Bin Meng wrote: > The stored 'blk' value is overwritten to 'size / 512' before it can > be used in usb_stor_set_max_xfer_blk(). This is not what we want. > In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we > should simply assign 'size' to the upper limit. > > Reported-by: Coverity (CID: 167250) > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> Still failing https://travis-ci.org/marex/u-boot-usb/builds/280835848 > --- > > common/usb_storage.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/usb_storage.c b/common/usb_storage.c > index a57570b..a91b1c0 100644 > --- a/common/usb_storage.c > +++ b/common/usb_storage.c > @@ -964,7 +964,7 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev, > blk = 20; > } else { > if (size > USHRT_MAX * 512) > - blk = USHRT_MAX; > + size = USHRT_MAX * 512; > blk = size / 512; > } > #endif >
Hi Marek, On Thu, Sep 28, 2017 at 11:24 PM, Marek Vasut <marex@denx.de> wrote: > On 09/28/2017 06:50 AM, Bin Meng wrote: >> The stored 'blk' value is overwritten to 'size / 512' before it can >> be used in usb_stor_set_max_xfer_blk(). This is not what we want. >> In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we >> should simply assign 'size' to the upper limit. >> >> Reported-by: Coverity (CID: 167250) >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > > Still failing > https://travis-ci.org/marex/u-boot-usb/builds/280835848 > This fixes the Coverity issue, not the 'make tests' issue. Please hold on apply the xHCI patchset and when the fix is ready I will send v2. >> --- >> >> common/usb_storage.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/common/usb_storage.c b/common/usb_storage.c >> index a57570b..a91b1c0 100644 >> --- a/common/usb_storage.c >> +++ b/common/usb_storage.c >> @@ -964,7 +964,7 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev, >> blk = 20; >> } else { >> if (size > USHRT_MAX * 512) >> - blk = USHRT_MAX; >> + size = USHRT_MAX * 512; >> blk = size / 512; >> } >> #endif >> Regards, Bin
On 09/29/2017 01:36 AM, Bin Meng wrote: > Hi Marek, > > On Thu, Sep 28, 2017 at 11:24 PM, Marek Vasut <marex@denx.de> wrote: >> On 09/28/2017 06:50 AM, Bin Meng wrote: >>> The stored 'blk' value is overwritten to 'size / 512' before it can >>> be used in usb_stor_set_max_xfer_blk(). This is not what we want. >>> In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we >>> should simply assign 'size' to the upper limit. >>> >>> Reported-by: Coverity (CID: 167250) >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> >> Still failing >> https://travis-ci.org/marex/u-boot-usb/builds/280835848 >> > > This fixes the Coverity issue, not the 'make tests' issue. Please hold > on apply the xHCI patchset and when the fix is ready I will send v2. Can't you send me fix on top of current set ? If not, OK, tell me what to drop and what to pick. Thanks >>> --- >>> >>> common/usb_storage.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/common/usb_storage.c b/common/usb_storage.c >>> index a57570b..a91b1c0 100644 >>> --- a/common/usb_storage.c >>> +++ b/common/usb_storage.c >>> @@ -964,7 +964,7 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev, >>> blk = 20; >>> } else { >>> if (size > USHRT_MAX * 512) >>> - blk = USHRT_MAX; >>> + size = USHRT_MAX * 512; >>> blk = size / 512; >>> } >>> #endif >>> > > Regards, > Bin >
Hi Marek, On Fri, Sep 29, 2017 at 10:30 AM, Marek Vasut <marex@denx.de> wrote: > On 09/29/2017 01:36 AM, Bin Meng wrote: >> Hi Marek, >> >> On Thu, Sep 28, 2017 at 11:24 PM, Marek Vasut <marex@denx.de> wrote: >>> On 09/28/2017 06:50 AM, Bin Meng wrote: >>>> The stored 'blk' value is overwritten to 'size / 512' before it can >>>> be used in usb_stor_set_max_xfer_blk(). This is not what we want. >>>> In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we >>>> should simply assign 'size' to the upper limit. >>>> >>>> Reported-by: Coverity (CID: 167250) >>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> >>> Still failing >>> https://travis-ci.org/marex/u-boot-usb/builds/280835848 >>> >> >> This fixes the Coverity issue, not the 'make tests' issue. Please hold >> on apply the xHCI patchset and when the fix is ready I will send v2. > > Can't you send me fix on top of current set ? If not, OK, tell me what > to drop and what to pick. > OK, will do. Regards, Bin
On 09/29/2017 11:26 AM, Bin Meng wrote: > Hi Marek, > > On Fri, Sep 29, 2017 at 10:30 AM, Marek Vasut <marex@denx.de> wrote: >> On 09/29/2017 01:36 AM, Bin Meng wrote: >>> Hi Marek, >>> >>> On Thu, Sep 28, 2017 at 11:24 PM, Marek Vasut <marex@denx.de> wrote: >>>> On 09/28/2017 06:50 AM, Bin Meng wrote: >>>>> The stored 'blk' value is overwritten to 'size / 512' before it can >>>>> be used in usb_stor_set_max_xfer_blk(). This is not what we want. >>>>> In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we >>>>> should simply assign 'size' to the upper limit. >>>>> >>>>> Reported-by: Coverity (CID: 167250) >>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>> >>>> Still failing >>>> https://travis-ci.org/marex/u-boot-usb/builds/280835848 >>>> >>> >>> This fixes the Coverity issue, not the 'make tests' issue. Please hold >>> on apply the xHCI patchset and when the fix is ready I will send v2. >> >> Can't you send me fix on top of current set ? If not, OK, tell me what >> to drop and what to pick. >> > > OK, will do. Thanks!
Hi Marek, On Fri, Sep 29, 2017 at 5:36 PM, Marek Vasut <marex@denx.de> wrote: > On 09/29/2017 11:26 AM, Bin Meng wrote: >> Hi Marek, >> >> On Fri, Sep 29, 2017 at 10:30 AM, Marek Vasut <marex@denx.de> wrote: >>> On 09/29/2017 01:36 AM, Bin Meng wrote: >>>> Hi Marek, >>>> >>>> On Thu, Sep 28, 2017 at 11:24 PM, Marek Vasut <marex@denx.de> wrote: >>>>> On 09/28/2017 06:50 AM, Bin Meng wrote: >>>>>> The stored 'blk' value is overwritten to 'size / 512' before it can >>>>>> be used in usb_stor_set_max_xfer_blk(). This is not what we want. >>>>>> In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we >>>>>> should simply assign 'size' to the upper limit. >>>>>> >>>>>> Reported-by: Coverity (CID: 167250) >>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>> >>>>> Still failing >>>>> https://travis-ci.org/marex/u-boot-usb/builds/280835848 >>>>> >>>> >>>> This fixes the Coverity issue, not the 'make tests' issue. Please hold >>>> on apply the xHCI patchset and when the fix is ready I will send v2. >>> >>> Can't you send me fix on top of current set ? If not, OK, tell me what >>> to drop and what to pick. >>> >> >> OK, will do. > > Thanks! > I just sent additional patches (http://patchwork.ozlabs.org/project/uboot/list/?series=5892) to fix the 'make tests' issues. Please do the following on the u-boot-usb/master branch: 1). Drop http://git.denx.de/?p=u-boot/u-boot-usb.git;a=commit;h=198db64035a589ef19920b2cec58419b4b338ce7 2). Drop http://git.denx.de/?p=u-boot/u-boot-usb.git;a=commit;h=9a462a3af216d77fee689f119419506218531f77 3). Apply http://patchwork.ozlabs.org/project/uboot/list/?series=5892 on top of u-boot-usb/master Then everything should be fine. Regards, Bin
On 10/01/2017 03:18 PM, Bin Meng wrote: > Hi Marek, > > On Fri, Sep 29, 2017 at 5:36 PM, Marek Vasut <marex@denx.de> wrote: >> On 09/29/2017 11:26 AM, Bin Meng wrote: >>> Hi Marek, >>> >>> On Fri, Sep 29, 2017 at 10:30 AM, Marek Vasut <marex@denx.de> wrote: >>>> On 09/29/2017 01:36 AM, Bin Meng wrote: >>>>> Hi Marek, >>>>> >>>>> On Thu, Sep 28, 2017 at 11:24 PM, Marek Vasut <marex@denx.de> wrote: >>>>>> On 09/28/2017 06:50 AM, Bin Meng wrote: >>>>>>> The stored 'blk' value is overwritten to 'size / 512' before it can >>>>>>> be used in usb_stor_set_max_xfer_blk(). This is not what we want. >>>>>>> In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we >>>>>>> should simply assign 'size' to the upper limit. >>>>>>> >>>>>>> Reported-by: Coverity (CID: 167250) >>>>>>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>>>>> >>>>>> Still failing >>>>>> https://travis-ci.org/marex/u-boot-usb/builds/280835848 >>>>>> >>>>> >>>>> This fixes the Coverity issue, not the 'make tests' issue. Please hold >>>>> on apply the xHCI patchset and when the fix is ready I will send v2. >>>> >>>> Can't you send me fix on top of current set ? If not, OK, tell me what >>>> to drop and what to pick. >>>> >>> >>> OK, will do. >> >> Thanks! >> > > I just sent additional patches > (http://patchwork.ozlabs.org/project/uboot/list/?series=5892) to fix > the 'make tests' issues. > > Please do the following on the u-boot-usb/master branch: > > 1). Drop http://git.denx.de/?p=u-boot/u-boot-usb.git;a=commit;h=198db64035a589ef19920b2cec58419b4b338ce7 > 2). Drop http://git.denx.de/?p=u-boot/u-boot-usb.git;a=commit;h=9a462a3af216d77fee689f119419506218531f77 > 3). Apply http://patchwork.ozlabs.org/project/uboot/list/?series=5892 > on top of u-boot-usb/master > > Then everything should be fine. Build tests pass, PR out, thanks !
diff --git a/common/usb_storage.c b/common/usb_storage.c index a57570b..a91b1c0 100644 --- a/common/usb_storage.c +++ b/common/usb_storage.c @@ -964,7 +964,7 @@ static void usb_stor_set_max_xfer_blk(struct usb_device *udev, blk = 20; } else { if (size > USHRT_MAX * 512) - blk = USHRT_MAX; + size = USHRT_MAX * 512; blk = size / 512; } #endif
The stored 'blk' value is overwritten to 'size / 512' before it can be used in usb_stor_set_max_xfer_blk(). This is not what we want. In fact, when 'size' exceeds the upper limit (USHRT_MAX * 512), we should simply assign 'size' to the upper limit. Reported-by: Coverity (CID: 167250) Signed-off-by: Bin Meng <bmeng.cn@gmail.com> --- common/usb_storage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)