Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-27 Thread Evgeny Yakovlev 
Great, thanks!

> 27 июня 2016 г., в 7:51, Tian, Feng <feng.t...@intel.com> написал(а):
> 
> Thanks, Yakovlev.
>  
> If there is no objection, I will commit and push these two patches to git 
> repo with your RB and mine.
>  
> Thanks
> Feng
>  
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
> Sent: Friday, June 24, 2016 9:35 PM
> To: Tian, Feng <feng.t...@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
>  
> Hi! We verified that xHCI with your patch correctly handles handshaking with 
> the problem device. 
>  
> 2016-06-14 3:42 GMT+03:00 Tian, Feng <feng.t...@intel.com>:
> Thanks for your info, Yakovlev.
> 
> Did you see problem with XHCI? If yes, I can provide a patch and could you 
> help verify it?
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com]
> Sent: Monday, June 13, 2016 5:29 PM
> To: Tian, Feng <feng.t...@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi Feng.
> 
> Sorry, i was AFK for the past week.
> 
> We have encountered this with Platronix USB headset. This device was 
> reporting *USB Interface* descriptor sizes 1 byte larger than what the spec 
> says. Headset was simply plugged in when we booted our firmware and 
> UsbCreateDesc failed to parse that and returned NULL which eventually crashed 
> the firmware later (i will try and send a separate patch to fix that). We 
> fixed UsbCreateDesc and device enumeration went fine, all descriptors it 
> sends are correct except for this odd size field.
> 
> Evgeny
> 
> > On 13 Jun 2016, at 05:32, Tian, Feng <feng.t...@intel.com> wrote:
> >
> > Hi, Yakovlev
> >
> > Any response on this?
> >
> > Thanks
> > Feng
> >
> > -----Original Message-----
> > From: Tian, Feng
> > Sent: Monday, June 6, 2016 9:27 AM
> > To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
> > Cc: Tian, Feng <feng.t...@intel.com>
> > Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> > length check
> >
> > Hi, Evgenv
> >
> > Thanks for your contribution, Evgenv
> >
> > Could you let me know what error case you met? Is the Device Desc or Config 
> > Desc larger than expected or other descs?
> >
> > Why I ask this question is because it would impact Xhci driver's 
> > implementation in which we store some desc contents at internal. So looks 
> > like XHCI driver also needs to be updated.
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> > Evgeny Yakovlev
> > Sent: Sunday, June 5, 2016 10:29 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> > check
> >
> > According to spec if the length of a descriptor is smaller than what the 
> > specification defines, then the host shall ignore it.
> > However if the size is greater than expected the host will ignore the extra 
> > bytes and start looking for the next descriptor at the end of actual length 
> > returned. Original check did not handle the latter case correctly and only 
> > allowed descriptors with lengths exactly as defined in specification.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
> > ---
> > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> > b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > index 5b8b1aa..fba60da 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > @@ -199,8 +199,8 @@ UsbCreateDesc (
> > }
> >   }
> >
> > -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> > -  (Head->Type != Type) || (Head->Len != DescLen)) {
> > +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> > +  (Head->Type != Type) || (Head->Len < DescLen)) {
> > DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> > return NULL;
> >   }
> > --
> > 2.7.4 (Apple Git-66)
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
>  
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-26 Thread Tian, Feng 
Thanks, Yakovlev.

If there is no objection, I will commit and push these two patches to git repo 
with your RB and mine.

Thanks
Feng

From: Evgeny Yakovlev [mailto:insorei...@gmail.com]
Sent: Friday, June 24, 2016 9:35 PM
To: Tian, Feng <feng.t...@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

Hi! We verified that xHCI with your patch correctly handles handshaking with 
the problem device.

2016-06-14 3:42 GMT+03:00 Tian, Feng 
<feng.t...@intel.com<mailto:feng.t...@intel.com>>:
Thanks for your info, Yakovlev.

Did you see problem with XHCI? If yes, I can provide a patch and could you help 
verify it?

Thanks
Feng

-Original Message-
From: Evgeny Yakovlev [mailto:insorei...@gmail.com<mailto:insorei...@gmail.com>]
Sent: Monday, June 13, 2016 5:29 PM
To: Tian, Feng <feng.t...@intel.com<mailto:feng.t...@intel.com>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

Hi Feng.

Sorry, i was AFK for the past week.

We have encountered this with Platronix USB headset. This device was reporting 
*USB Interface* descriptor sizes 1 byte larger than what the spec says. Headset 
was simply plugged in when we booted our firmware and UsbCreateDesc failed to 
parse that and returned NULL which eventually crashed the firmware later (i 
will try and send a separate patch to fix that). We fixed UsbCreateDesc and 
device enumeration went fine, all descriptors it sends are correct except for 
this odd size field.

Evgeny

> On 13 Jun 2016, at 05:32, Tian, Feng 
> <feng.t...@intel.com<mailto:feng.t...@intel.com>> wrote:
>
> Hi, Yakovlev
>
> Any response on this?
>
> Thanks
> Feng
>
> -Original Message-
> From: Tian, Feng
> Sent: Monday, June 6, 2016 9:27 AM
> To: Evgeny Yakovlev <insorei...@gmail.com<mailto:insorei...@gmail.com>>; 
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Tian, Feng <feng.t...@intel.com<mailto:feng.t...@intel.com>>
> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
>
> Hi, Evgenv
>
> Thanks for your contribution, Evgenv
>
> Could you let me know what error case you met? Is the Device Desc or Config 
> Desc larger than expected or other descs?
>
> Why I ask this question is because it would impact Xhci driver's 
> implementation in which we store some desc contents at internal. So looks 
> like XHCI driver also needs to be updated.
>
> Thanks
> Feng
>
> -Original Message-
> From: edk2-devel 
> [mailto:edk2-devel-boun...@lists.01.org<mailto:edk2-devel-boun...@lists.01.org>]
>  On Behalf Of Evgeny Yakovlev
> Sent: Sunday, June 5, 2016 10:29 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> check
>
> According to spec if the length of a descriptor is smaller than what the 
> specification defines, then the host shall ignore it.
> However if the size is greater than expected the host will ignore the extra 
> bytes and start looking for the next descriptor at the end of actual length 
> returned. Original check did not handle the latter case correctly and only 
> allowed descriptors with lengths exactly as defined in specification.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evgeny Yakovlev 
> <insorei...@gmail.com<mailto:insorei...@gmail.com>>
> ---
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> index 5b8b1aa..fba60da 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> @@ -199,8 +199,8 @@ UsbCreateDesc (
> }
>   }
>
> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> -  (Head->Type != Type) || (Head->Len != DescLen)) {
> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> +  (Head->Type != Type) || (Head->Len < DescLen)) {
> DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> return NULL;
>   }
> --
> 2.7.4 (Apple Git-66)
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-24 Thread Evgeny Yakovlev 
Hi! We verified that xHCI with your patch correctly handles handshaking
with the problem device.

2016-06-14 3:42 GMT+03:00 Tian, Feng <feng.t...@intel.com>:

> Thanks for your info, Yakovlev.
>
> Did you see problem with XHCI? If yes, I can provide a patch and could you
> help verify it?
>
> Thanks
> Feng
>
> -Original Message-
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com]
> Sent: Monday, June 13, 2016 5:29 PM
> To: Tian, Feng <feng.t...@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor
> length check
>
> Hi Feng.
>
> Sorry, i was AFK for the past week.
>
> We have encountered this with Platronix USB headset. This device was
> reporting *USB Interface* descriptor sizes 1 byte larger than what the spec
> says. Headset was simply plugged in when we booted our firmware and
> UsbCreateDesc failed to parse that and returned NULL which eventually
> crashed the firmware later (i will try and send a separate patch to fix
> that). We fixed UsbCreateDesc and device enumeration went fine, all
> descriptors it sends are correct except for this odd size field.
>
> Evgeny
>
> > On 13 Jun 2016, at 05:32, Tian, Feng <feng.t...@intel.com> wrote:
> >
> > Hi, Yakovlev
> >
> > Any response on this?
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: Tian, Feng
> > Sent: Monday, June 6, 2016 9:27 AM
> > To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
> > Cc: Tian, Feng <feng.t...@intel.com>
> > Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor
> length check
> >
> > Hi, Evgenv
> >
> > Thanks for your contribution, Evgenv
> >
> > Could you let me know what error case you met? Is the Device Desc or
> Config Desc larger than expected or other descs?
> >
> > Why I ask this question is because it would impact Xhci driver's
> implementation in which we store some desc contents at internal. So looks
> like XHCI driver also needs to be updated.
> >
> > Thanks
> > Feng
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Evgeny Yakovlev
> > Sent: Sunday, June 5, 2016 10:29 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor
> length check
> >
> > According to spec if the length of a descriptor is smaller than what the
> specification defines, then the host shall ignore it.
> > However if the size is greater than expected the host will ignore the
> extra bytes and start looking for the next descriptor at the end of actual
> length returned. Original check did not handle the latter case correctly
> and only allowed descriptors with lengths exactly as defined in
> specification.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
> > ---
> > MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > index 5b8b1aa..fba60da 100644
> > --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> > @@ -199,8 +199,8 @@ UsbCreateDesc (
> > }
> >   }
> >
> > -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> > -  (Head->Type != Type) || (Head->Len != DescLen)) {
> > +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> > +  (Head->Type != Type) || (Head->Len < DescLen)) {
> > DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> > return NULL;
> >   }
> > --
> > 2.7.4 (Apple Git-66)
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-14 Thread Tian, Feng 
Hi, Yakovlev

Here is the patch. I suspect XHCI may work without this patch because your case 
is the interface descriptor length is larger than expected. But this patch 
should be able to handle the less-than case.

Many thanks for your help.

Your patch on UsbBus could get my RB. Reviewed-by: Feng Tian 
<feng.t...@intel.com>.

I will help check it later.

Thanks
Feng

-Original Message-
From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
Sent: Wednesday, June 15, 2016 2:15 AM
To: Tian, Feng <feng.t...@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

Hi! We never tried that device with xHCI, however we probably can try to do 
that and test your patch. 

> On 14 Jun 2016, at 03:42, Tian, Feng <feng.t...@intel.com> wrote:
> 
> Thanks for your info, Yakovlev.
> 
> Did you see problem with XHCI? If yes, I can provide a patch and could you 
> help verify it?
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
> Sent: Monday, June 13, 2016 5:29 PM
> To: Tian, Feng <feng.t...@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi Feng. 
> 
> Sorry, i was AFK for the past week.
> 
> We have encountered this with Platronix USB headset. This device was 
> reporting *USB Interface* descriptor sizes 1 byte larger than what the spec 
> says. Headset was simply plugged in when we booted our firmware and 
> UsbCreateDesc failed to parse that and returned NULL which eventually crashed 
> the firmware later (i will try and send a separate patch to fix that). We 
> fixed UsbCreateDesc and device enumeration went fine, all descriptors it 
> sends are correct except for this odd size field.
> 
> Evgeny
> 
>> On 13 Jun 2016, at 05:32, Tian, Feng <feng.t...@intel.com> wrote:
>> 
>> Hi, Yakovlev
>> 
>> Any response on this? 
>> 
>> Thanks
>> Feng
>> 
>> -Original Message-----
>> From: Tian, Feng 
>> Sent: Monday, June 6, 2016 9:27 AM
>> To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
>> Cc: Tian, Feng <feng.t...@intel.com>
>> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
>> length check
>> 
>> Hi, Evgenv
>> 
>> Thanks for your contribution, Evgenv
>> 
>> Could you let me know what error case you met? Is the Device Desc or Config 
>> Desc larger than expected or other descs?
>> 
>> Why I ask this question is because it would impact Xhci driver's 
>> implementation in which we store some desc contents at internal. So looks 
>> like XHCI driver also needs to be updated.
>> 
>> Thanks
>> Feng
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Evgeny Yakovlev
>> Sent: Sunday, June 5, 2016 10:29 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
>> check
>> 
>> According to spec if the length of a descriptor is smaller than what the 
>> specification defines, then the host shall ignore it.
>> However if the size is greater than expected the host will ignore the extra 
>> bytes and start looking for the next descriptor at the end of actual length 
>> returned. Original check did not handle the latter case correctly and only 
>> allowed descriptors with lengths exactly as defined in specification.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
>> ---
>> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
>> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
>> index 5b8b1aa..fba60da 100644
>> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
>> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
>> @@ -199,8 +199,8 @@ UsbCreateDesc (
>>}
>>  }
>> 
>> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
>> -  (Head->Type != Type) || (Head->Len != DescLen)) {
>> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
>> +  (Head->Type != Type) || (Head->Len < DescLen)) {
>>DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
>>return NULL;
>>  }
>> --
>> 2.7.4 (Apple Git-66)
>> 
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-14 Thread Evgeny Yakovlev 
Hi! We never tried that device with xHCI, however we probably can try to do 
that and test your patch. 

> On 14 Jun 2016, at 03:42, Tian, Feng <feng.t...@intel.com> wrote:
> 
> Thanks for your info, Yakovlev.
> 
> Did you see problem with XHCI? If yes, I can provide a patch and could you 
> help verify it?
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
> Sent: Monday, June 13, 2016 5:29 PM
> To: Tian, Feng <feng.t...@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi Feng. 
> 
> Sorry, i was AFK for the past week.
> 
> We have encountered this with Platronix USB headset. This device was 
> reporting *USB Interface* descriptor sizes 1 byte larger than what the spec 
> says. Headset was simply plugged in when we booted our firmware and 
> UsbCreateDesc failed to parse that and returned NULL which eventually crashed 
> the firmware later (i will try and send a separate patch to fix that). We 
> fixed UsbCreateDesc and device enumeration went fine, all descriptors it 
> sends are correct except for this odd size field.
> 
> Evgeny
> 
>> On 13 Jun 2016, at 05:32, Tian, Feng <feng.t...@intel.com> wrote:
>> 
>> Hi, Yakovlev
>> 
>> Any response on this? 
>> 
>> Thanks
>> Feng
>> 
>> -Original Message-
>> From: Tian, Feng 
>> Sent: Monday, June 6, 2016 9:27 AM
>> To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
>> Cc: Tian, Feng <feng.t...@intel.com>
>> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
>> length check
>> 
>> Hi, Evgenv
>> 
>> Thanks for your contribution, Evgenv
>> 
>> Could you let me know what error case you met? Is the Device Desc or Config 
>> Desc larger than expected or other descs?
>> 
>> Why I ask this question is because it would impact Xhci driver's 
>> implementation in which we store some desc contents at internal. So looks 
>> like XHCI driver also needs to be updated.
>> 
>> Thanks
>> Feng
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Evgeny Yakovlev
>> Sent: Sunday, June 5, 2016 10:29 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
>> check
>> 
>> According to spec if the length of a descriptor is smaller than what the 
>> specification defines, then the host shall ignore it.
>> However if the size is greater than expected the host will ignore the extra 
>> bytes and start looking for the next descriptor at the end of actual length 
>> returned. Original check did not handle the latter case correctly and only 
>> allowed descriptors with lengths exactly as defined in specification.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
>> ---
>> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
>> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
>> index 5b8b1aa..fba60da 100644
>> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
>> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
>> @@ -199,8 +199,8 @@ UsbCreateDesc (
>>}
>>  }
>> 
>> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
>> -  (Head->Type != Type) || (Head->Len != DescLen)) {
>> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
>> +  (Head->Type != Type) || (Head->Len < DescLen)) {
>>DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
>>return NULL;
>>  }
>> --
>> 2.7.4 (Apple Git-66)
>> 
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-13 Thread Tian, Feng 
Thanks for your info, Yakovlev.

Did you see problem with XHCI? If yes, I can provide a patch and could you help 
verify it?

Thanks
Feng

-Original Message-
From: Evgeny Yakovlev [mailto:insorei...@gmail.com] 
Sent: Monday, June 13, 2016 5:29 PM
To: Tian, Feng <feng.t...@intel.com>
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

Hi Feng. 

Sorry, i was AFK for the past week.

We have encountered this with Platronix USB headset. This device was reporting 
*USB Interface* descriptor sizes 1 byte larger than what the spec says. Headset 
was simply plugged in when we booted our firmware and UsbCreateDesc failed to 
parse that and returned NULL which eventually crashed the firmware later (i 
will try and send a separate patch to fix that). We fixed UsbCreateDesc and 
device enumeration went fine, all descriptors it sends are correct except for 
this odd size field.

Evgeny

> On 13 Jun 2016, at 05:32, Tian, Feng <feng.t...@intel.com> wrote:
> 
> Hi, Yakovlev
> 
> Any response on this? 
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Tian, Feng 
> Sent: Monday, June 6, 2016 9:27 AM
> To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.t...@intel.com>
> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi, Evgenv
> 
> Thanks for your contribution, Evgenv
> 
> Could you let me know what error case you met? Is the Device Desc or Config 
> Desc larger than expected or other descs?
> 
> Why I ask this question is because it would impact Xhci driver's 
> implementation in which we store some desc contents at internal. So looks 
> like XHCI driver also needs to be updated.
> 
> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evgeny 
> Yakovlev
> Sent: Sunday, June 5, 2016 10:29 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> check
> 
> According to spec if the length of a descriptor is smaller than what the 
> specification defines, then the host shall ignore it.
> However if the size is greater than expected the host will ignore the extra 
> bytes and start looking for the next descriptor at the end of actual length 
> returned. Original check did not handle the latter case correctly and only 
> allowed descriptors with lengths exactly as defined in specification.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
> ---
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> index 5b8b1aa..fba60da 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> @@ -199,8 +199,8 @@ UsbCreateDesc (
> }
>   }
> 
> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> -  (Head->Type != Type) || (Head->Len != DescLen)) {
> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> +  (Head->Type != Type) || (Head->Len < DescLen)) {
> DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> return NULL;
>   }
> --
> 2.7.4 (Apple Git-66)
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-13 Thread Evgeny Yakovlev 
Hi Feng. 

Sorry, i was AFK for the past week.

We have encountered this with Platronix USB headset. This device was reporting 
*USB Interface* descriptor sizes 1 byte larger than what the spec says. Headset 
was simply plugged in when we booted our firmware and UsbCreateDesc failed to 
parse that and returned NULL which eventually crashed the firmware later (i 
will try and send a separate patch to fix that). We fixed UsbCreateDesc and 
device enumeration went fine, all descriptors it sends are correct except for 
this odd size field.

Evgeny

> On 13 Jun 2016, at 05:32, Tian, Feng <feng.t...@intel.com> wrote:
> 
> Hi, Yakovlev
> 
> Any response on this? 
> 
> Thanks
> Feng
> 
> -Original Message-
> From: Tian, Feng 
> Sent: Monday, June 6, 2016 9:27 AM
> To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.t...@intel.com>
> Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor 
> length check
> 
> Hi, Evgenv
> 
> Thanks for your contribution, Evgenv
> 
> Could you let me know what error case you met? Is the Device Desc or Config 
> Desc larger than expected or other descs?
> 
> Why I ask this question is because it would impact Xhci driver's 
> implementation in which we store some desc contents at internal. So looks 
> like XHCI driver also needs to be updated.
> 
> Thanks
> Feng
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evgeny 
> Yakovlev
> Sent: Sunday, June 5, 2016 10:29 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
> check
> 
> According to spec if the length of a descriptor is smaller than what the 
> specification defines, then the host shall ignore it.
> However if the size is greater than expected the host will ignore the extra 
> bytes and start looking for the next descriptor at the end of actual length 
> returned. Original check did not handle the latter case correctly and only 
> allowed descriptors with lengths exactly as defined in specification.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
> ---
> MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
> b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> index 5b8b1aa..fba60da 100644
> --- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> +++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
> @@ -199,8 +199,8 @@ UsbCreateDesc (
> }
>   }
> 
> -  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
> -  (Head->Type != Type) || (Head->Len != DescLen)) {
> +  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
> +  (Head->Type != Type) || (Head->Len < DescLen)) {
> DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
> return NULL;
>   }
> --
> 2.7.4 (Apple Git-66)
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-12 Thread Tian, Feng 
Hi, Yakovlev

Any response on this? 

Thanks
Feng

-Original Message-
From: Tian, Feng 
Sent: Monday, June 6, 2016 9:27 AM
To: Evgeny Yakovlev <insorei...@gmail.com>; edk2-devel@lists.01.org
Cc: Tian, Feng <feng.t...@intel.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

Hi, Evgenv

Thanks for your contribution, Evgenv

Could you let me know what error case you met? Is the Device Desc or Config 
Desc larger than expected or other descs?

Why I ask this question is because it would impact Xhci driver's implementation 
in which we store some desc contents at internal. So looks like XHCI driver 
also needs to be updated.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evgeny 
Yakovlev
Sent: Sunday, June 5, 2016 10:29 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

According to spec if the length of a descriptor is smaller than what the 
specification defines, then the host shall ignore it.
However if the size is greater than expected the host will ignore the extra 
bytes and start looking for the next descriptor at the end of actual length 
returned. Original check did not handle the latter case correctly and only 
allowed descriptors with lengths exactly as defined in specification.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Evgeny Yakovlev <insorei...@gmail.com>
---
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
index 5b8b1aa..fba60da 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
@@ -199,8 +199,8 @@ UsbCreateDesc (
 }
   }
 
-  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
-  (Head->Type != Type) || (Head->Len != DescLen)) {
+  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
+  (Head->Type != Type) || (Head->Len < DescLen)) {
 DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
 return NULL;
   }
--
2.7.4 (Apple Git-66)

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length check

2016-06-05 Thread Tian, Feng 
Hi, Evgenv

Thanks for your contribution, Evgenv

Could you let me know what error case you met? Is the Device Desc or Config 
Desc larger than expected or other descs?

Why I ask this question is because it would impact Xhci driver's implementation 
in which we store some desc contents at internal. So looks like XHCI driver 
also needs to be updated.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evgeny 
Yakovlev
Sent: Sunday, June 5, 2016 10:29 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg/UsbBusDxe: Fixed USB descriptor length 
check

According to spec if the length of a descriptor is smaller than what the 
specification defines, then the host shall ignore it.
However if the size is greater than expected the host will ignore the extra 
bytes and start looking for the next descriptor at the end of actual length 
returned. Original check did not handle the latter case correctly and only 
allowed descriptors with lengths exactly as defined in specification.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Evgeny Yakovlev 
---
 MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c 
b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
index 5b8b1aa..fba60da 100644
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
@@ -199,8 +199,8 @@ UsbCreateDesc (
 }
   }
 
-  if ((Len <= Offset)  || (Len < Offset + DescLen) ||
-  (Head->Type != Type) || (Head->Len != DescLen)) {
+  if ((Len <= Offset)  || (Len < Offset + Head->Len) ||
+  (Head->Type != Type) || (Head->Len < DescLen)) {
 DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
 return NULL;
   }
--
2.7.4 (Apple Git-66)

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel