On Fri, Aug 26, 2022 at 08:30:50AM +0200, Marek Szyprowski wrote:
> On 11.08.2022 18:06, Alan Stern wrote:
> > On Thu, Aug 11, 2022 at 09:31:34AM +0200, Marek Szyprowski wrote:
> >> On 10.08.2022 21:33, Alan Stern wrote:
> >>> On Mon, Aug 08, 2022 at 04:26:49PM -0400, Alan Stern wrote:
> >>>> On Mon, Aug 08, 2022 at 04:57:35PM +0200, Marek Szyprowski wrote:
> >>>>> This patch landed recently in linux-next as commit 2191c00855b0 ("USB:
> >>>>> gadget: Fix use-after-free Read in usb_udc_uevent()"). Unfortunately it
> >>>>> fixes the issue by introducing another one. It doesn't look very
> >>>>> probable, but it would be nice to fix it to make the lock dependency
> >>>>> checker happy.
> >>>> Indeed.
> >>>> I suspect the problem is that udc_lock is held for too long. Probably it
> >>>> should be released during the calls to udc->driver->bind and
> >>>> udc->driver->unbind.
> >>>>
> >>>> Getting this right will require some careful study. Marek, if I send you
> >>>> a patch later, will you be able to test it?
> >>> Here's a patch for you to try, when you have the chance. It reduces the
> >>> scope of udc_lock to cover only the fields it's supposed to protect and
> >>> changes the locking in a few other places.
> >>>
> >>> There's still the possibility of a locking cycle, because udc_lock is
> >>> held in the ->disconnect pathway. It's very hard to know whether that
> >>> might cause any trouble; it depends on how the function drivers handle
> >>> disconnections.
> >> It looks this fixed the issue I've reported. I've checked it on all my
> >> test systems and none reported any issue related to the udc.
> >>
> >> Feel free to add:
> >>
> >> Reported-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >>
> >> Tested-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> > Thanks for the quick testing. I'll submit the patch when the current
> > merge window ends.
>
> Gentle ping for the final patch...
On its way. It turns out that the hardest part of this patch is writing
up the descriptions and justifications in the Changelog. :-(
(Part of the reason for the delay is that I usually wait until the -rc2
or -rc3 release before starting to work on a new kernel version...)
I think in the end there is more work still to do. In particular, the
gadget core doesn't seem to be careful about not connecting (i.e., not
turning on the D+ pullup) when no driver is bound to the gadget. Those
actions need to be synchronized somehow.
Alan Stern
