[PATCH v2] usb: misc: uss720: properly clean up reference in uss720_probe()

[PATCH v2] usb: misc: uss720: properly clean up reference in uss720_probe()

[Greg Kroah-Hartman]

If get_1284_register() fails, the usb device reference count is
incorrect and needs to be properly dropped before returning.

Cc: stable <stable@kernel.org>
Assisted-by: gkh_clanker_2000
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 v2: properly clean up the previously allocated resources by jumping to
     the error path and not just retrning directly

 drivers/usb/misc/uss720.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
index ec8bd968c4de..54107bf280df 100644
--- a/drivers/usb/misc/uss720.c
+++ b/drivers/usb/misc/uss720.c
@@ -735,8 +735,10 @@ static int uss720_probe(struct usb_interface *intf,
 	 * here. */
 	ret = get_1284_register(pp, 0, &reg, GFP_KERNEL);
 	dev_dbg(&intf->dev, "reg: %7ph\n", priv->reg);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		usb_put_dev(usbdev);
+		goto probe_abort;
+	}
 
 	ret = usb_find_last_int_in_endpoint(interface, &epd);
 	if (!ret) {
-- 
2.53.0

Re: [PATCH v2] usb: misc: uss720: properly clean up reference in uss720_probe()

[Oliver Neukum]

Hi,

On 23.02.26 12:17, Greg Kroah-Hartman wrote:
> If get_1284_register() fails, the usb device reference count is
> incorrect and needs to be properly dropped before returning.
> 
> Cc: stable <stable@kernel.org>
> Assisted-by: gkh_clanker_2000
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>   v2: properly clean up the previously allocated resources by jumping to
>       the error path and not just retrning directly
> 
>   drivers/usb/misc/uss720.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
> index ec8bd968c4de..54107bf280df 100644
> --- a/drivers/usb/misc/uss720.c
> +++ b/drivers/usb/misc/uss720.c
> @@ -735,8 +735,10 @@ static int uss720_probe(struct usb_interface *intf,
>   	 * here. */
>   	ret = get_1284_register(pp, 0, &reg, GFP_KERNEL);
>   	dev_dbg(&intf->dev, "reg: %7ph\n", priv->reg);
> -	if (ret < 0)
> -		return ret;
> +	if (ret < 0) {
> +		usb_put_dev(usbdev);
> +		goto probe_abort;

This jumps to probe_abort, which calls destroy_priv() indirectly
via kref_put(). Either that works, which would mean that you must
_not_ do a second usb_put_dev() or it does not, in which case the
earlier error handling for parport_register_port() is incorrect.

Either way, there is a problem.

	Regards
		Oliver

Re: [PATCH v2] usb: misc: uss720: properly clean up reference in uss720_probe()

[Greg Kroah-Hartman]

On Mon, Feb 23, 2026 at 12:28:29PM +0100, Oliver Neukum wrote:
> Hi,
> 
> On 23.02.26 12:17, Greg Kroah-Hartman wrote:
> > If get_1284_register() fails, the usb device reference count is
> > incorrect and needs to be properly dropped before returning.
> > 
> > Cc: stable <stable@kernel.org>
> > Assisted-by: gkh_clanker_2000
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >   v2: properly clean up the previously allocated resources by jumping to
> >       the error path and not just retrning directly
> > 
> >   drivers/usb/misc/uss720.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/usb/misc/uss720.c b/drivers/usb/misc/uss720.c
> > index ec8bd968c4de..54107bf280df 100644
> > --- a/drivers/usb/misc/uss720.c
> > +++ b/drivers/usb/misc/uss720.c
> > @@ -735,8 +735,10 @@ static int uss720_probe(struct usb_interface *intf,
> >   	 * here. */
> >   	ret = get_1284_register(pp, 0, &reg, GFP_KERNEL);
> >   	dev_dbg(&intf->dev, "reg: %7ph\n", priv->reg);
> > -	if (ret < 0)
> > -		return ret;
> > +	if (ret < 0) {
> > +		usb_put_dev(usbdev);
> > +		goto probe_abort;
> 
> This jumps to probe_abort, which calls destroy_priv() indirectly
> via kref_put(). Either that works, which would mean that you must
> _not_ do a second usb_put_dev() or it does not, in which case the
> earlier error handling for parport_register_port() is incorrect.
> 
> Either way, there is a problem.

Argh, I missed that part of the error path, you are right.  The existing
code is wrong for the earlier cleanup logic, I'll go fix that instead,
thanks for the review.

It's tricky stuff like this which makes me can't wait for rust to be
possible for USB drivers, someday...

thanks,

greg k-h

Re: [PATCH v2] usb: misc: uss720: properly clean up reference in uss720_probe()

[Oliver Neukum]

On 23.02.26 13:14, Greg Kroah-Hartman wrote:
  
> It's tricky stuff like this which makes me can't wait for rust to be
> possible for USB drivers, someday...

Well, as you wish to touch upon that topic ...

I am afraid this is not a problem of language. Rust
solves the issue of object life time. Unfortunately
that is useless in this case. I know we all wish
to solve the issue, but let me explain.

Now, before you either start despairing or get angry, we
need to look at concepts. It seems to my you're looking at
the problem in terms of life time and basically
something that can be solved by reference counting
respectively life time rules.
That is unfortunately not true. Our problem with USB
drivers is a question of binding. probe() does not mean
that a device has been plugged in, nor does disconnect()
mean that a device has gone away. It means that the binding
between an interface and a driver is requested respectively
goes away. Hotplug is merely the most common cause of
these requests.

Nevertheless you have to cease using the interface as
disconnect() is called. References cannot change that.
There are two reasons for that

a) there is no object representing the binding. It is technically
a pointer not a data structure. There is nothing to refcount
and no object whose lifetime you can specify. There is a variable
that is changing
b) there is a state transition, not a life time question.
There is an event that changes the state of a binding if you
will.

Sorry you don't like this, but this is a design issue, not
a language issue.

	Regards
		Oliver

Re: [PATCH v2] usb: misc: uss720: properly clean up reference in uss720_probe()

[Greg Kroah-Hartman]

On Mon, Feb 23, 2026 at 01:55:55PM +0100, Oliver Neukum wrote:
> On 23.02.26 13:14, Greg Kroah-Hartman wrote:
> > It's tricky stuff like this which makes me can't wait for rust to be
> > possible for USB drivers, someday...
> 
> Well, as you wish to touch upon that topic ...
> 
> I am afraid this is not a problem of language. Rust
> solves the issue of object life time. Unfortunately
> that is useless in this case. I know we all wish
> to solve the issue, but let me explain.
> 
> Now, before you either start despairing or get angry, we
> need to look at concepts. It seems to my you're looking at
> the problem in terms of life time and basically
> something that can be solved by reference counting
> respectively life time rules.
> That is unfortunately not true. Our problem with USB
> drivers is a question of binding. probe() does not mean
> that a device has been plugged in, nor does disconnect()
> mean that a device has gone away. It means that the binding
> between an interface and a driver is requested respectively
> goes away. Hotplug is merely the most common cause of
> these requests.
> 
> Nevertheless you have to cease using the interface as
> disconnect() is called. References cannot change that.
> There are two reasons for that
> 
> a) there is no object representing the binding. It is technically
> a pointer not a data structure. There is nothing to refcount
> and no object whose lifetime you can specify. There is a variable
> that is changing
> b) there is a state transition, not a life time question.
> There is an event that changes the state of a binding if you
> will.
> 
> Sorry you don't like this, but this is a design issue, not
> a language issue.

It's not a choice of me liking it or not, but I think if you look at how
the driver model binds through rust to drivers, most/all of what you
refer to above is already worked out as there the issues here are with
the driver handling resources that it thinks it still has access to (or
forgets about.)  We have working bindings for PCI and other busses
already for rust, and this is handled there pretty cleanly, I see no
reason why USB would be any different.

thanks,

greg k-h