| Message ID | 20230125072138.21925-1-quic_ugoswami@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] usb: gadget: configfs: Restrict symlink creation is UDC already binded | expand |
Hi Udipto, W dniu 25.01.2023 o 08:21, Udipto Goswami pisze: > During enumeration or composition switch,a userspace process > agnostic of the conventions of configs can try to create function > symlinks even after the UDC is bound to current config which is > not correct. Potentially it can create duplicates within the > current config. > > Prevent this by adding a check if udc_name already exists then bail > out of cfg_link. > > Fixes: 88af8bbe4ef7 ("usb: gadget: the start of the configfs interface") > Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com> > --- > v2: Fixed spelling mistakes in commit text. > > drivers/usb/gadget/configfs.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c > index 78e7353e397b..434e49d29c50 100644 > --- a/drivers/usb/gadget/configfs.c > +++ b/drivers/usb/gadget/configfs.c > @@ -455,6 +455,11 @@ static int config_usb_cfg_link( > } > } > > + if (gi->composite.gadget_driver.udc_name) { > + ret = -EINVAL; > + goto out; > + } > + If we want to introduce this kind of check, I'd say it should be done as the very first thing in this function - in particular before traversing two linked lists (&gi->available_func, &cfg->func_list). And, you probably want to keep the word "PATCH" in the title, like this: [PATCH v2] usb: gadget: configfs: ..... Regards, Andrzej > f = usb_get_function(fi); > if (IS_ERR(f)) { > ret = PTR_ERR(f);
Hi Andrzej, On 1/25/23 6:21 PM, Andrzej Pietrasiewicz wrote: > Hi Udipto, > > W dniu 25.01.2023 o 08:21, Udipto Goswami pisze: >> During enumeration or composition switch,a userspace process >> agnostic of the conventions of configs can try to create function >> symlinks even after the UDC is bound to current config which is >> not correct. Potentially it can create duplicates within the >> current config. >> >> Prevent this by adding a check if udc_name already exists then bail >> out of cfg_link. >> >> Fixes: 88af8bbe4ef7 ("usb: gadget: the start of the configfs interface") >> Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com> >> --- >> v2: Fixed spelling mistakes in commit text. >> >> drivers/usb/gadget/configfs.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/usb/gadget/configfs.c >> b/drivers/usb/gadget/configfs.c >> index 78e7353e397b..434e49d29c50 100644 >> --- a/drivers/usb/gadget/configfs.c >> +++ b/drivers/usb/gadget/configfs.c >> @@ -455,6 +455,11 @@ static int config_usb_cfg_link( >> } >> } >> + if (gi->composite.gadget_driver.udc_name) { >> + ret = -EINVAL; >> + goto out; >> + } >> + > > If we want to introduce this kind of check, I'd say it should be done > as the very first thing in this function - in particular before > traversing two linked lists (&gi->available_func, &cfg->func_list). > > And, you probably want to keep the word "PATCH" in the title, like this: > > [PATCH v2] usb: gadget: configfs: ..... sure I'll address these in v3. > > Regards, > > Andrzej > >> f = usb_get_function(fi); >> if (IS_ERR(f)) { >> ret = PTR_ERR(f); > Thanks, -Udipto
diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c index 78e7353e397b..434e49d29c50 100644 --- a/drivers/usb/gadget/configfs.c +++ b/drivers/usb/gadget/configfs.c @@ -455,6 +455,11 @@ static int config_usb_cfg_link( } } + if (gi->composite.gadget_driver.udc_name) { + ret = -EINVAL; + goto out; + } + f = usb_get_function(fi); if (IS_ERR(f)) { ret = PTR_ERR(f);
During enumeration or composition switch,a userspace process agnostic of the conventions of configs can try to create function symlinks even after the UDC is bound to current config which is not correct. Potentially it can create duplicates within the current config. Prevent this by adding a check if udc_name already exists then bail out of cfg_link. Fixes: 88af8bbe4ef7 ("usb: gadget: the start of the configfs interface") Signed-off-by: Udipto Goswami <quic_ugoswami@quicinc.com> --- v2: Fixed spelling mistakes in commit text. drivers/usb/gadget/configfs.c | 5 +++++ 1 file changed, 5 insertions(+)