[v2,8/8] usb: gadget: uvc: configfs: Prevent format changes after linking header

[v2,8/8] usb: gadget: uvc: configfs: Prevent format changes after linking header

[Laurent Pinchart]

From: Joel Pepper <joel.pepper@rwth-aachen.de>

While checks are in place to avoid attributes and children of a format
being manipulated after the format is linked into the streaming header,
the linked flag was never actually set, invalidating the protections.

Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
---
 drivers/usb/gadget/function/uvc_configfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index b8763343dcae..799dc32c5bc7 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -928,6 +928,7 @@ static int uvcg_streaming_header_allow_link(struct config_item *src,
 	format_ptr->fmt = target_fmt;
 	list_add_tail(&format_ptr->entry, &src_hdr->formats);
 	++src_hdr->num_fmt;
+	++target_fmt->linked;
 
 out:
 	mutex_unlock(&opts->lock);
@@ -965,6 +966,8 @@ static void uvcg_streaming_header_drop_link(struct config_item *src,
 			break;
 		}
 
+	--target_fmt->linked;
+
 out:
 	mutex_unlock(&opts->lock);
 	mutex_unlock(su_mutex);

[v2,8/8] usb: gadget: uvc: configfs: Prevent format changes after linking header

[Kieran Bingham]

Hi Laurent, Joel,

On 01/08/18 22:55, Laurent Pinchart wrote:
> From: Joel Pepper <joel.pepper@rwth-aachen.de>
> 
> While checks are in place to avoid attributes and children of a format
> being manipulated after the format is linked into the streaming header,
> the linked flag was never actually set, invalidating the protections.

That explains what's wrong, but not what we do about it. How about adding:

"Update the flag as appropriate in the header link calls."

> 
> Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>

Missing and S-o-B from Laurent here? (I guess that will be added later?)

Otherwise,

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>



> ---
>  drivers/usb/gadget/function/uvc_configfs.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index b8763343dcae..799dc32c5bc7 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -928,6 +928,7 @@ static int uvcg_streaming_header_allow_link(struct config_item *src,
>  	format_ptr->fmt = target_fmt;
>  	list_add_tail(&format_ptr->entry, &src_hdr->formats);
>  	++src_hdr->num_fmt;
> +	++target_fmt->linked;
>  
>  out:
>  	mutex_unlock(&opts->lock);
> @@ -965,6 +966,8 @@ static void uvcg_streaming_header_drop_link(struct config_item *src,
>  			break;
>  		}
>  
> +	--target_fmt->linked;
> +
>  out:
>  	mutex_unlock(&opts->lock);
>  	mutex_unlock(su_mutex);
>

[v2,8/8] usb: gadget: uvc: configfs: Prevent format changes after linking header

[Laurent Pinchart]

Hi Kieran,

On Monday, 24 September 2018 15:50:45 EEST Kieran Bingham wrote:
> On 01/08/18 22:55, Laurent Pinchart wrote:
> > From: Joel Pepper <joel.pepper@rwth-aachen.de>
> > 
> > While checks are in place to avoid attributes and children of a format
> > being manipulated after the format is linked into the streaming header,
> > the linked flag was never actually set, invalidating the protections.
> 
> That explains what's wrong, but not what we do about it. How about adding:
> 
> "Update the flag as appropriate in the header link calls."

Sounds good to me, I'll update that.

> > Signed-off-by: Joel Pepper <joel.pepper@rwth-aachen.de>
> 
> Missing and S-o-B from Laurent here? (I guess that will be added later?)

I haven't modified the patch, so haven't added my SoB to send it to the 
mailing list. I'll add it before issuing the pull request.

> Otherwise,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > ---
> > 
> >  drivers/usb/gadget/function/uvc_configfs.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/usb/gadget/function/uvc_configfs.c
> > b/drivers/usb/gadget/function/uvc_configfs.c index
> > b8763343dcae..799dc32c5bc7 100644
> > --- a/drivers/usb/gadget/function/uvc_configfs.c
> > +++ b/drivers/usb/gadget/function/uvc_configfs.c
> > @@ -928,6 +928,7 @@ static int uvcg_streaming_header_allow_link(struct
> > config_item *src,> 
> >  	format_ptr->fmt = target_fmt;
> >  	list_add_tail(&format_ptr->entry, &src_hdr->formats);
> >  	++src_hdr->num_fmt;
> > 
> > +	++target_fmt->linked;
> > 
> >  out:
> >  	mutex_unlock(&opts->lock);
> > 
> > @@ -965,6 +966,8 @@ static void uvcg_streaming_header_drop_link(struct
> > config_item *src,> 
> >  			break;
> >  		
> >  		}
> > 
> > +	--target_fmt->linked;
> > +
> > 
> >  out:
> >  	mutex_unlock(&opts->lock);
> >  	mutex_unlock(su_mutex);