USB: gadget: Fix UAF on refcount inside gadgetfs_bind()
Subsystems (2)
PATCH v2 1/1 USB: gadget: Fix UAF on refcount inside gadgetfs_bind()
When usb_ep_alloc_request() fails, gadgetfs_bind() takes the following error path:
if (!dev->req)
goto enomem;
// ...
enomem:
gadgetfs_unbind (gadget); return -ENOMEM;
In normal case, get_dev() will be called to increment refcounter from gadget->dev->count but in error case, this increment is skipped and gadgetfs_unbind() will decrement it via put_dev().
Going down the error path leads to a refcount imbalance which will cause UAF in close()/umount() operations due to refcount dropping below 0.
Fix this by moving get_dev() above the error paths so that refcounter is incremented before the put_dev() call.
---
Changes from v1:
Removed syzkaller reference in description
Added Reviewed-by: tag.
drivers/usb/gadget/legacy/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/gadget/legacy/inode.c +1 -1
@@ -1679,6 +1679,7 @@ static int gadgetfs_bind(struct usb_gadget *gadget, }set_gadget_data (gadget, dev); +get_dev (dev); dev->gadget = gadget; gadget->ep0->driver_data = dev;
@@ -1696,7 +1697,6 @@ static int gadgetfs_bind(struct usb_gadget *gadget,spin_lock_irq (&dev->lock); dev->state =STATE_DEV_UNCONNECTED ;spin_unlock_irq (&dev->lock); -get_dev (dev);return 0 ; enomem:
Signature · 1 line
2.43.0
Greg KH
maintainerQuoted 62 lines · Rafael Alejandro Diaz Cruz
When
usb_ep_alloc_request()fails,gadgetfs_bind()takes the following error path:if (!dev->req)
goto enomem;// ...
enomem:
gadgetfs_unbind (gadget); return -ENOMEM;In normal case,
get_dev()will be called to increment refcounter fromgadget->dev->countbut in error case, this increment is skipped andgadgetfs_unbind()will decrement it viaput_dev().Going down the error path leads to a refcount imbalance which will cause UAF in close()/umount() operations due to refcount dropping below 0.
Fix this by moving
get_dev()above the error paths so that refcounter is incremented before theput_dev()call.Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")Reported-by [email protected]Signed-off-by Rafael Alejandro Diaz Cruz <[email protected]>Reviewed-by Alan Stern <[email protected]>---
Changes from v1:
Removed syzkaller reference in descriptionAdded Reviewed-by: tag.
drivers/usb/gadget/legacy/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/usb/gadget/legacy/inode.c +1
-1@@ -1679,6 +1679,7 @@ static int gadgetfs_bind(struct usb_gadget *gadget, }set_gadget_data (gadget, dev); +get_dev (dev); dev->gadget = gadget; gadget->ep0->driver_data = dev;@@ -1696,7 +1697,6 @@ static int gadgetfs_bind(struct usb_gadget *gadget,spin_lock_irq (&dev->lock); dev->state =STATE_DEV_UNCONNECTED ;spin_unlock_irq (&dev->lock); -get_dev (dev);return 0 ; enomem:Signature · 1 line
2.43.0
I think
https://lore.kernel.org/r/[email protected]
was send before yours.
thanks,
greg k-h