lkml.org 
[lkml]   [2023]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH] misc: hisi_hikey_usb: Fix use after free bug in hisi_hikey_usb_remove due to race condition
Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月13日周四 18:55写道:
>
> Hi, Zheng
>
> On Thu, 13 Apr 2023 at 16:08, Zheng Hacker <hackerzheng666@gmail.com> 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.

In practice, you can just use rmmod command to simulate the unplug
movement, which will also trigger the hisi_hikey_usb_remove if there
is a real USB device.

If there's some other help I can provide, please feel free to let me know.

Thanks again for your effort.

Best regards,
Zheng

[1] https://www.usenix.org/conference/usenixsecurity21/presentation/lee-yoochan

> Thanks,
> Yongqin Liu
> > Zheng Hacker <hackerzheng666@gmail.com> 于2023年3月14日周二 09:01写道:
> > >
> > > John Stultz <jstultz@google.com> 于2023年3月14日周二 03:57写道:
> > > >
> > > > On Sun, Mar 12, 2023 at 7:53 AM Zheng Wang <zyytlz.wz@163.com> wrote:
> > > > >
> > > > > In hisi_hikey_usb_probe, it called hisi_hikey_usb_of_role_switch
> > > > > and bound &hisi_hikey_usb->work with relay_set_role_switch.
> > > > > When it calls hub_usb_role_switch_set, it will finally call
> > > > > schedule_work to start the work.
> > > > >
> > > > > When we call hisi_hikey_usb_remove to remove the driver, there
> > > > > may be a sequence as follows:
> > > > >
> > > > > Fix it by finishing the work before cleanup in hisi_hikey_usb_remove.
> > > > >
> > > > > CPU0 CPU1
> > > > >
> > > > > |relay_set_role_switch
> > > > > hisi_hikey_usb_remove|
> > > > > usb_role_switch_put|
> > > > > usb_role_switch_release |
> > > > > kfree(sw) |
> > > > > | usb_role_switch_set_role
> > > > > | //use
> > > > >
> > > > > Fixes: 7a6ff4c4cbc3 ("misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960")
> > > > > Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
> > > > > ---
> > > > > drivers/misc/hisi_hikey_usb.c | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
> > > > > index 2165ec35a343..26fc895c4418 100644
> > > > > --- a/drivers/misc/hisi_hikey_usb.c
> > > > > +++ b/drivers/misc/hisi_hikey_usb.c
> > > > > @@ -242,6 +242,7 @@ static int hisi_hikey_usb_probe(struct platform_device *pdev)
> > > > > static int hisi_hikey_usb_remove(struct platform_device *pdev)
> > > > > {
> > > > > struct hisi_hikey_usb *hisi_hikey_usb = platform_get_drvdata(pdev);
> > > > > + cancel_work_sync(&hisi_hikey_usb->work);
> > > > >
> > > > > if (hisi_hikey_usb->hub_role_sw) {
> > > > > usb_role_switch_unregister(hisi_hikey_usb->hub_role_sw);
> > > >
> > > > Looks sane to me.
> > > > Pulling in Sumit and YongQin as they have hardware and can test with it.
> > > >
> > > Hi John,
> > >
> > > Thanks for your reply. Thank Sumit and YongQin for being willing to
> > > test the solution with their hardware.
> > >
> > > Best regards,
> > > Zheng
>
>
>
> --
> Best Regards,
> Yongqin Liu
> ---------------------------------------------------------------
> #mailing list
> linaro-android@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-android

\
 
 \ /
  Last update: 2023-04-13 19:06    [W:0.044 / U:0.288 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site