linux-kernel - Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting) response behaviour

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4c33f742-cdb7-92b2-9596-efe99443f89c@ti.com>
Date:   Tue, 7 Mar 2017 10:41:06 +0200
From:   Roger Quadros <rogerq@...com>
To:     Laurent Pinchart <laurent.pinchart@...asonboard.com>
CC:     <balbi@...nel.org>, <b-liu@...com>, <nsekhar@...com>,
        <linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] uvc-gadget: Fix Set Interface (alternate setting)
 response behaviour

Laurent,

On 06/03/17 22:50, Laurent Pinchart wrote:
> Hi Roger,
> 
> Thank you for the patch.
> 
> On Friday 03 Mar 2017 13:17:15 Roger Quadros wrote:
>> On alternate setting change, webcam gadget sends us a UVC_EVENT_STREAMON
>> or UVC_EVENT_STREAMOFF event. It expects delayed status response on
>> STREAMON event only but doesn't expect us to send that response over USB.
>> It sends the delayed response when we issue the VIDIOC_STREAMON ioctl.
>>
>> So we must not send UVCIOC_SEND_RESPONSE ioctl in these cases that too
>> with invalid response length.
> 
> The commit message only explains why we should not call UVCIOC_SEND_RESPONSE 
> in response to a STREAMON event, but not why we shouldn't either in response 
> to a STREAMOFF event. The patch is correct changing both, but I propose 
> wording the above two paragraphs as follows.
> 
> "uvc-gadget: Do not send Set Interface (alternate setting) response twice
> 
> On alternate setting change, the webcam gadget sends us a UVC_EVENT_STREAMON 
> or UVC_EVENT_STREAMOFF event. In the first case, the driver will issue a 
> delayed status response automatically when we call the VIDIOC_STREAMON ioctl. 
> In the second case, the driver sends the status response immediately. We must 
> thus not send the status response manually with UVCIOC_SEND_RESPONSE in any of 
> those cases."
> 
> If you're fine with that I'll change the message when applying, there's no 
> need to resend the patch.

I'm fine with your suggested commit message. Thanks.

> 
>> Without this, the ISO streaming doesn't work if host application
>> (e.g. luvcview) is closed and restarted.
>> On dwc3 gadget controller it was resulting in Buffer Expiry error on
>> the ISO endpoint.
>>
>> Signed-off-by: Roger Quadros <rogerq@...com>
>> ---
>>  uvc-gadget.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/uvc-gadget.c b/uvc-gadget.c
>> index 9ef315c..4d59ab8 100644
>> --- a/uvc-gadget.c
>> +++ b/uvc-gadget.c
>> @@ -597,12 +597,12 @@ uvc_events_process(struct uvc_device *dev)
>>  	case UVC_EVENT_STREAMON:
>>  		uvc_video_reqbufs(dev, 4);
>>  		uvc_video_stream(dev, 1);
>> -		break;
>> +		return;
>>
>>  	case UVC_EVENT_STREAMOFF:
>>  		uvc_video_stream(dev, 0);
>>  		uvc_video_reqbufs(dev, 0);
>> -		break;
>> +		return;
>>  	}
>>
>>  	ioctl(dev->fd, UVCIOC_SEND_RESPONSE, &resp);
> 

-- 
cheers,
-roger