Re: [PATCH] misc: hisi_hikey_usb: Fix use after free bug in hisi_hikey_usb_remove due to race condition

[Yongqin Liu]

Hi, Zheng

Sorry for the late reply.

I tested this change with Android build based on the ACK
android-mainline branch.
The hisi_hikey_usb module could not be removed with error like this:
    console:/ # rmmod -f hisi_hikey_usb
    rmmod: failed to unload hisi_hikey_usb: Try again
    1|console:/ #
Sorry I am not able to reproduce any problem without this commit,
but I do not see any problem with this change applied either.

If there is any specific things you want to check, please feel free let me know

Thanks,
Yongqin Liu


On Fri, 14 Apr 2023 at 00:46, Zheng Hacker <hackerzheng666@xxxxxxxxx> wrote:
>
> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 于2023年4月13日周四 23:56写道：
> >
> > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote:
> > > Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 于2023年4月13日周四 20:48写道：
> > > >
> > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote:
> > > > > Yongqin Liu <yongqin.liu@xxxxxxxxxx> 于2023年4月13日周四 18:55写道：
> > > > > >
> > > > > > Hi, Zheng
> > > > > >
> > > > > > On Thu, 13 Apr 2023 at 16:08, Zheng Hacker <hackerzheng666@xxxxxxxxx> wrote:
> > > > > > >
> > > > > > > Friendly ping about the bug.
> > > > > >
> > > > > > Sorry, wasn't aware of this message before,
> > > > > >
> > > > > > Could you please help share the instructions to reproduce the problem
> > > > > > this change fixes?
> > > > > >
> > > > >
> > > > > Hi Yongqin,
> > > > >
> > > > > Thanks for your reply. This bug is found by static analysis. There is no PoC.
> > > > >
> > > > > >From my personal experience, triggering race condition bugs stably in
> > > > > the kernel needs some tricks.
> > > > > For example, you can insert some sleep-time code to slow down the
> > > > > thread until the related object is freed.
> > > > > Besides, you can use gdb to control the time window. Also, there are
> > > > > some other tricks as [1] said.
> > > > >
> > > > > As for the reproduction, this attack vector requires that the attacker
> > > > > can physically access the device.
> > > > > When he/she unplugs the usb, the remove function is triggered, and if
> > > > > the set callback is invoked, there might be a race condition.
> > > >
> > > > How does the removal of the USB device trigger a platform device
> > > > removal?
> > >
> > > Sorry I made a mistake. The USB device usually calls disconnect
> > > callback when it's unpluged.
> >
> > Yes, but you are changing the platform device disconnect, not the USB
> > device disconnect.
> >
> > > What I want to express here is When the driver-related device(here
> > > it's USB GPIO Hub) was removed, the remove function is triggered.
> >
> > And is this a patform device on a USB device?  If so, that's a bigger
> > problem that we need to fix as that is not allowed.
>
> No this is not a platform  device on a USB device.
>
> >
> > But in looking at the code, it does not seem to be that at all, this is
> > just a "normal" platform device.  So how can it ever be removed from the
> > system?  (and no, unloading the driver doesn't count, that can never
> > happen on a normal machine.)
> >
>
> Yes, I finally figured out your meaning. I know it's hard to unplug
> the platform device
> directly. All I want to express is that it's a theoretical method
> except  rmmod. I think it's better to fix the bug. But if the
> developers think it's practically impossible, I think there's no need
> to take further action.
>
> Sorry for wasting your time and thanks for your explanation.
>
> Best regards,
> Zheng
>
> > thanks,
> >
> > greg k-h



--
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@xxxxxxxxxxxxxxxx
http://lists.linaro.org/mailman/listinfo/linaro-android