Re: [PATCH] usb: gadget: f_ncm: Always set current gadget in ncm_bind()

[Carlos Llamas <cmllamas@google.com>]

On Fri, Oct 20, 2023 at 05:33:24PM +0200, Hardik Gajjar wrote:
> Previously, gadget assignment to the net device occurred exclusively
> during the initial binding attempt.
> 
> Nevertheless, the gadget pointer could change during bind/unbind
> cycles due to various conditions, including the unloading/loading
> of the UDC device driver or the detachment/reconnection of an
> OTG-capable USB hub device.
> 
> This patch relocates the gether_set_gadget() function out from
> ncm_opts->bound condition check, ensuring that the correct gadget
> is assigned during each bind request.

Hi, sorry to dig out this old thread, but I'm seeing some issues in a
downstream kernel that seem relevant to this patch.

It seems to me that swapping the parent device like this might be a bit
more complex that it appears. When the register_netdev() is skipped for
the new parent gadgets it misses a crucial device_get(), and this leads
to an unbalanced reference count.

During tear-down (e.g. after device mode switch), the reference count on
the device unexpectedly reaches zero and the gadget is kfreed. Note this
doesn't happen with the gadget that was initially bound thanks to the
register_netdev() call.

Unfortunatelly, releasing the gadget device after ->unbind() leaves a
dangling pointer in netdev->parent. And certain operations such as a
netlink dump() will attempt to derreference the netdev->parent as cause
a use-after-free.

I checked this behavior by tracing several paths and using KASAN. And
looking at the upstream code, I believe the issue is also present and
was introduced by this patch.

Please let me know if anyone has some ideas on how to proceed, or if you
need me to run some specific tests or potential fixes. I would be happy
to help.

--
Carlos Llamas