diff mbox series

[U-Boot] usb: gadget: composite: fix NULL pointer when a non standard request is received

Message ID 1521102857-6497-1-git-send-email-patrice.chotard@st.com
State Accepted
Commit d57ed4d9f2cb6abc9438104f321b4fe3bc829d57
Delegated to: Lukasz Majewski
Headers show
Series [U-Boot] usb: gadget: composite: fix NULL pointer when a non standard request is received | expand

Commit Message

Patrice CHOTARD March 15, 2018, 8:34 a.m. UTC
From: Christophe Kerello <christophe.kerello@st.com>

In case usb configuration is unknown (cdev->config == NULL), non standard
request should not be processed.
Remove also the cdev->config check below which will never happen.

This issue was seen using ums feature.

Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
---
 drivers/usb/gadget/composite.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Patrice CHOTARD April 26, 2018, 1:13 p.m. UTC | #1
Hi

It's a gentle reminder as this patch is present on mailing list since 5 
weeks without any feedback.

Thanks

On 03/15/2018 09:34 AM, patrice.chotard@st.com wrote:
> From: Christophe Kerello <christophe.kerello@st.com>
> 
> In case usb configuration is unknown (cdev->config == NULL), non standard
> request should not be processed.
> Remove also the cdev->config check below which will never happen.
> 
> This issue was seen using ums feature.
> 
> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> ---
>   drivers/usb/gadget/composite.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> index d0ee7847b98d..a87639def974 100644
> --- a/drivers/usb/gadget/composite.c
> +++ b/drivers/usb/gadget/composite.c
> @@ -838,6 +838,9 @@ unknown:
>   			ctrl->bRequestType, ctrl->bRequest,
>   			w_value, w_index, w_length);
>   
> +		if (!cdev->config)
> +			goto done;
> +
>   		/*
>   		 * functions always handle their interfaces and endpoints...
>   		 * punt other recipients (other, WUSB, ...) to the current
> @@ -882,7 +885,7 @@ unknown:
>   			value = f->setup(f, ctrl);
>   		else {
>   			c = cdev->config;
> -			if (c && c->setup)
> +			if (c->setup)
>   				value = c->setup(c, ctrl);
>   		}
>   
>
Marek Vasut April 26, 2018, 1:15 p.m. UTC | #2
On 04/26/2018 03:13 PM, Patrice CHOTARD wrote:
> Hi
> 
> It's a gentle reminder as this patch is present on mailing list since 5 
> weeks without any feedback.

This should've been like a third non-gentle ping, seriously, two weeks
without feedback is already sucky. Time to buzz Lukasz, although he is
probably busy with personal stuff these days.

Applied

> Thanks
> 
> On 03/15/2018 09:34 AM, patrice.chotard@st.com wrote:
>> From: Christophe Kerello <christophe.kerello@st.com>
>>
>> In case usb configuration is unknown (cdev->config == NULL), non standard
>> request should not be processed.
>> Remove also the cdev->config check below which will never happen.
>>
>> This issue was seen using ums feature.
>>
>> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>> ---
>>   drivers/usb/gadget/composite.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
>> index d0ee7847b98d..a87639def974 100644
>> --- a/drivers/usb/gadget/composite.c
>> +++ b/drivers/usb/gadget/composite.c
>> @@ -838,6 +838,9 @@ unknown:
>>   			ctrl->bRequestType, ctrl->bRequest,
>>   			w_value, w_index, w_length);
>>   
>> +		if (!cdev->config)
>> +			goto done;
>> +
>>   		/*
>>   		 * functions always handle their interfaces and endpoints...
>>   		 * punt other recipients (other, WUSB, ...) to the current
>> @@ -882,7 +885,7 @@ unknown:
>>   			value = f->setup(f, ctrl);
>>   		else {
>>   			c = cdev->config;
>> -			if (c && c->setup)
>> +			if (c->setup)
>>   				value = c->setup(c, ctrl);
>>   		}
>>
Lukasz Majewski April 26, 2018, 1:17 p.m. UTC | #3
Hi Patrice,

> Hi
> 
> It's a gentle reminder as this patch is present on mailing list since
> 5 weeks without any feedback.

Deepest apologizes for the delay.

I will test this patch - as some Samsung SoCs may use this feature and
let you know. 

> 
> Thanks
> 
> On 03/15/2018 09:34 AM, patrice.chotard@st.com wrote:
> > From: Christophe Kerello <christophe.kerello@st.com>
> > 
> > In case usb configuration is unknown (cdev->config == NULL), non
> > standard request should not be processed.
> > Remove also the cdev->config check below which will never happen.
> > 
> > This issue was seen using ums feature.
> > 
> > Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
> > Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
> > ---
> >   drivers/usb/gadget/composite.c | 5 ++++-
> >   1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/gadget/composite.c
> > b/drivers/usb/gadget/composite.c index d0ee7847b98d..a87639def974
> > 100644 --- a/drivers/usb/gadget/composite.c
> > +++ b/drivers/usb/gadget/composite.c
> > @@ -838,6 +838,9 @@ unknown:
> >   			ctrl->bRequestType, ctrl->bRequest,
> >   			w_value, w_index, w_length);
> >   
> > +		if (!cdev->config)
> > +			goto done;
> > +
> >   		/*
> >   		 * functions always handle their interfaces and
> > endpoints...
> >   		 * punt other recipients (other, WUSB, ...) to
> > the current @@ -882,7 +885,7 @@ unknown:
> >   			value = f->setup(f, ctrl);
> >   		else {
> >   			c = cdev->config;
> > -			if (c && c->setup)
> > +			if (c->setup)
> >   				value = c->setup(c, ctrl);
> >   		}
> >   
> >  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Patrice CHOTARD April 27, 2018, 7:52 a.m. UTC | #4
Thanks Lukasz ;-)

On 04/26/2018 03:17 PM, Lukasz Majewski wrote:
> Hi Patrice,
> 
>> Hi
>>
>> It's a gentle reminder as this patch is present on mailing list since
>> 5 weeks without any feedback.
> 
> Deepest apologizes for the delay.
> 
> I will test this patch - as some Samsung SoCs may use this feature and
> let you know.
> 
>>
>> Thanks
>>
>> On 03/15/2018 09:34 AM, patrice.chotard@st.com wrote:
>>> From: Christophe Kerello <christophe.kerello@st.com>
>>>
>>> In case usb configuration is unknown (cdev->config == NULL), non
>>> standard request should not be processed.
>>> Remove also the cdev->config check below which will never happen.
>>>
>>> This issue was seen using ums feature.
>>>
>>> Signed-off-by: Christophe Kerello <christophe.kerello@st.com>
>>> Signed-off-by: Patrice Chotard <patrice.chotard@st.com>
>>> ---
>>>    drivers/usb/gadget/composite.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/gadget/composite.c
>>> b/drivers/usb/gadget/composite.c index d0ee7847b98d..a87639def974
>>> 100644 --- a/drivers/usb/gadget/composite.c
>>> +++ b/drivers/usb/gadget/composite.c
>>> @@ -838,6 +838,9 @@ unknown:
>>>    			ctrl->bRequestType, ctrl->bRequest,
>>>    			w_value, w_index, w_length);
>>>    
>>> +		if (!cdev->config)
>>> +			goto done;
>>> +
>>>    		/*
>>>    		 * functions always handle their interfaces and
>>> endpoints...
>>>    		 * punt other recipients (other, WUSB, ...) to
>>> the current @@ -882,7 +885,7 @@ unknown:
>>>    			value = f->setup(f, ctrl);
>>>    		else {
>>>    			c = cdev->config;
>>> -			if (c && c->setup)
>>> +			if (c->setup)
>>>    				value = c->setup(c, ctrl);
>>>    		}
>>>    
>>>   
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
>
diff mbox series

Patch

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index d0ee7847b98d..a87639def974 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -838,6 +838,9 @@  unknown:
 			ctrl->bRequestType, ctrl->bRequest,
 			w_value, w_index, w_length);
 
+		if (!cdev->config)
+			goto done;
+
 		/*
 		 * functions always handle their interfaces and endpoints...
 		 * punt other recipients (other, WUSB, ...) to the current
@@ -882,7 +885,7 @@  unknown:
 			value = f->setup(f, ctrl);
 		else {
 			c = cdev->config;
-			if (c && c->setup)
+			if (c->setup)
 				value = c->setup(c, ctrl);
 		}