drivers/misc/hisi_hikey_usb.c | 1 + 1 file changed, 1 insertion(+)
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);
--
2.25.1On 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.
thanks
-john
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
Friendly ping about the bug.
Thanks,
Zheng
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
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?
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
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
On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > 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. How does the removal of the USB device trigger a platform device removal? Are you sure this can be triggered by some other way other than manually unloading the driver? thanks, greg k-h
Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > 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. > > 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. 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. > > Are you sure this can be triggered by some other way other than manually > unloading the driver? As I didn't make a PoC, I'm not 100 percent sure about that. Best regards, Zheng > > thanks, > > greg k-h
On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote: > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > > 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. > > > > 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. 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.) thanks, greg k-h
Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道： > > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote: > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > > > 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. > > > > > > 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
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@gmail.com> wrote:
>
> Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道：
> >
> > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote:
> > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道：
> > > >
> > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote:
> > > > > 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.
> > > >
> > > > 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@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android
Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月18日周二 01:31写道： > > 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 > Hi Yongqin, Thanks for your testing. I have no more questions about the issue. Best regards, Zheng > Thanks, > Yongqin Liu > > > On Fri, 14 Apr 2023 at 00:46, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道： > > > > > > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote: > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > > > > > > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > > > > > 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. > > > > > > > > > > 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@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-android
Hi, Zheng
BTW, I just see cancel_delayed_work_sync is used in
the drivers/usb/common/usb-conn-gpio.c usb_conn_remove function.
https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/common/usb-conn-gpio.c#274
I know nothing about the cancel_delayed_work_sync and cancel_work_sync
functions,
just for your information in case cancel_delayed_work_sync might be
better to be used here.
Thanks,
Yongqin Liu
On Tue, 18 Apr 2023 at 21:18, Zheng Hacker <hackerzheng666@gmail.com> wrote:
>
> Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月18日周二 01:31写道：
> >
> > 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
> >
>
> Hi Yongqin,
>
> Thanks for your testing. I have no more questions about the issue.
>
> Best regards,
> Zheng
>
> > Thanks,
> > Yongqin Liu
> >
> >
> > On Fri, 14 Apr 2023 at 00:46, Zheng Hacker <hackerzheng666@gmail.com> wrote:
> > >
> > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道：
> > > >
> > > > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote:
> > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道：
> > > > > >
> > > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote:
> > > > > > > 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.
> > > > > >
> > > > > > 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@lists.linaro.org
> > http://lists.linaro.org/mailman/listinfo/linaro-android
--
Best Regards,
Yongqin Liu
---------------------------------------------------------------
#mailing list
linaro-android@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-android
Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月20日周四 14:31写道： > > Hi, Zheng > > BTW, I just see cancel_delayed_work_sync is used in > the drivers/usb/common/usb-conn-gpio.c usb_conn_remove function. > https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/common/usb-conn-gpio.c#274 > > I know nothing about the cancel_delayed_work_sync and cancel_work_sync > functions, > just for your information in case cancel_delayed_work_sync might be > better to be used here. > HI Yongqin, Thanks for your kind reminder. The cancel_delayed_work_sync is used with INIT_DELAYED_WORK and queue_delayed_work. This is used to start a work after some time while schedule_work means start it immediately. In this case, the &hisi_hikey_usb->work is initialized with INIT_WORK and scheduled with schedule_work. So I think we'd better use cancel_work_sync here. I'm also not so familiar with the code. If there's something wrong with it, please feel free to let me know. Best regards, Zheng > Thanks, > Yongqin Liu > On Tue, 18 Apr 2023 at 21:18, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月18日周二 01:31写道： > > > > > > 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 > > > > > > > Hi Yongqin, > > > > Thanks for your testing. I have no more questions about the issue. > > > > Best regards, > > Zheng > > > > > Thanks, > > > Yongqin Liu > > > > > > > > > On Fri, 14 Apr 2023 at 00:46, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道： > > > > > > > > > > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote: > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > > > > > > > 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. > > > > > > > > > > > > > > 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@lists.linaro.org > > > http://lists.linaro.org/mailman/listinfo/linaro-android > > > > -- > Best Regards, > Yongqin Liu > --------------------------------------------------------------- > #mailing list > linaro-android@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-android
HI, Zheng Thanks for the explanation! BTW, from what I tested, I am OK to have this change merged. Thanks, Yongqin Liu On Fri, 21 Apr 2023 at 10:35, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月20日周四 14:31写道： > > > > Hi, Zheng > > > > BTW, I just see cancel_delayed_work_sync is used in > > the drivers/usb/common/usb-conn-gpio.c usb_conn_remove function. > > https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/common/usb-conn-gpio.c#274 > > > > I know nothing about the cancel_delayed_work_sync and cancel_work_sync > > functions, > > just for your information in case cancel_delayed_work_sync might be > > better to be used here. > > > > HI Yongqin, > > Thanks for your kind reminder. The cancel_delayed_work_sync is used > with INIT_DELAYED_WORK and queue_delayed_work. > This is used to start a work after some time while schedule_work means > start it immediately. > > In this case, the &hisi_hikey_usb->work is initialized with INIT_WORK > and scheduled with schedule_work. So I think we'd better use > cancel_work_sync here. I'm also not so familiar with the code. If > there's something wrong with it, please feel free to let me know. > > Best regards, > Zheng > > > > Thanks, > > Yongqin Liu > > On Tue, 18 Apr 2023 at 21:18, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > > > Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月18日周二 01:31写道： > > > > > > > > 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 > > > > > > > > > > Hi Yongqin, > > > > > > Thanks for your testing. I have no more questions about the issue. > > > > > > Best regards, > > > Zheng > > > > > > > Thanks, > > > > Yongqin Liu > > > > > > > > > > > > On Fri, 14 Apr 2023 at 00:46, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道： > > > > > > > > > > > > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote: > > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > > > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > > > > > > > > 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. > > > > > > > > > > > > > > > > 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@lists.linaro.org > > > > http://lists.linaro.org/mailman/listinfo/linaro-android > > > > > > > > -- > > Best Regards, > > Yongqin Liu > > --------------------------------------------------------------- > > #mailing list > > linaro-android@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/linaro-android -- Best Regards, Yongqin Liu --------------------------------------------------------------- #mailing list linaro-android@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-android
Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月21日周五 23:42写道： > > HI, Zheng > > Thanks for the explanation! > BTW, from what I tested, I am OK to have this change merged. > Hi Yongqin, Thanks for your testing and review. Best regards, Zheng > Thanks, > Yongqin Liu > On Fri, 21 Apr 2023 at 10:35, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月20日周四 14:31写道： > > > > > > Hi, Zheng > > > > > > BTW, I just see cancel_delayed_work_sync is used in > > > the drivers/usb/common/usb-conn-gpio.c usb_conn_remove function. > > > https://android.googlesource.com/kernel/common/+/refs/heads/android-mainline/drivers/usb/common/usb-conn-gpio.c#274 > > > > > > I know nothing about the cancel_delayed_work_sync and cancel_work_sync > > > functions, > > > just for your information in case cancel_delayed_work_sync might be > > > better to be used here. > > > > > > > HI Yongqin, > > > > Thanks for your kind reminder. The cancel_delayed_work_sync is used > > with INIT_DELAYED_WORK and queue_delayed_work. > > This is used to start a work after some time while schedule_work means > > start it immediately. > > > > In this case, the &hisi_hikey_usb->work is initialized with INIT_WORK > > and scheduled with schedule_work. So I think we'd better use > > cancel_work_sync here. I'm also not so familiar with the code. If > > there's something wrong with it, please feel free to let me know. > > > > Best regards, > > Zheng > > > > > > > Thanks, > > > Yongqin Liu > > > On Tue, 18 Apr 2023 at 21:18, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > > > > > Yongqin Liu <yongqin.liu@linaro.org> 于2023年4月18日周二 01:31写道： > > > > > > > > > > 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 > > > > > > > > > > > > > Hi Yongqin, > > > > > > > > Thanks for your testing. I have no more questions about the issue. > > > > > > > > Best regards, > > > > Zheng > > > > > > > > > Thanks, > > > > > Yongqin Liu > > > > > > > > > > > > > > > On Fri, 14 Apr 2023 at 00:46, Zheng Hacker <hackerzheng666@gmail.com> wrote: > > > > > > > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 23:56写道： > > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 11:35:17PM +0800, Zheng Hacker wrote: > > > > > > > > Greg KH <gregkh@linuxfoundation.org> 于2023年4月13日周四 20:48写道： > > > > > > > > > > > > > > > > > > On Thu, Apr 13, 2023 at 07:12:07PM +0800, Zheng Hacker wrote: > > > > > > > > > > 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. > > > > > > > > > > > > > > > > > > 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@lists.linaro.org > > > > > http://lists.linaro.org/mailman/listinfo/linaro-android > > > > > > > > > > > > -- > > > Best Regards, > > > Yongqin Liu > > > --------------------------------------------------------------- > > > #mailing list > > > linaro-android@lists.linaro.org > > > http://lists.linaro.org/mailman/listinfo/linaro-android > > > > -- > Best Regards, > Yongqin Liu > --------------------------------------------------------------- > #mailing list > linaro-android@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-android
© 2016 - 2026 Red Hat, Inc.