| From 9313d139aa25e572d860f6f673b73a20f32d7f93 Mon Sep 17 00:00:00 2001 |
| From: Andrey Konovalov <andreyknvl@gmail.com> |
| Date: Wed, 4 Sep 2024 03:30:51 +0200 |
| Subject: usb: gadget: dummy_hcd: execute hrtimer callback in softirq context |
| |
| From: Andrey Konovalov <andreyknvl@gmail.com> |
| |
| commit 9313d139aa25e572d860f6f673b73a20f32d7f93 upstream. |
| |
| Commit a7f3813e589f ("usb: gadget: dummy_hcd: Switch to hrtimer transfer |
| scheduler") switched dummy_hcd to use hrtimer and made the timer's |
| callback be executed in the hardirq context. |
| |
| With that change, __usb_hcd_giveback_urb now gets executed in the hardirq |
| context, which causes problems for KCOV and KMSAN. |
| |
| One problem is that KCOV now is unable to collect coverage from |
| the USB code that gets executed from the dummy_hcd's timer callback, |
| as KCOV cannot collect coverage in the hardirq context. |
| |
| Another problem is that the dummy_hcd hrtimer might get triggered in the |
| middle of a softirq with KCOV remote coverage collection enabled, and that |
| causes a WARNING in KCOV, as reported by syzbot. (I sent a separate patch |
| to shut down this WARNING, but that doesn't fix the other two issues.) |
| |
| Finally, KMSAN appears to ignore tracking memory copying operations |
| that happen in the hardirq context, which causes false positive |
| kernel-infoleaks, as reported by syzbot. |
| |
| Change the hrtimer in dummy_hcd to execute the callback in the softirq |
| context. |
| |
| Reported-by: syzbot+2388cdaeb6b10f0c13ac@syzkaller.appspotmail.com |
| Closes: https://syzkaller.appspot.com/bug?extid=2388cdaeb6b10f0c13ac |
| Reported-by: syzbot+17ca2339e34a1d863aad@syzkaller.appspotmail.com |
| Closes: https://syzkaller.appspot.com/bug?extid=17ca2339e34a1d863aad |
| Reported-by: syzbot+c793a7eca38803212c61@syzkaller.appspotmail.com |
| Closes: https://syzkaller.appspot.com/bug?extid=c793a7eca38803212c61 |
| Reported-by: syzbot+1e6e0b916b211bee1bd6@syzkaller.appspotmail.com |
| Closes: https://syzkaller.appspot.com/bug?extid=1e6e0b916b211bee1bd6 |
| Reported-by: kernel test robot <oliver.sang@intel.com> |
| Closes: https://lore.kernel.org/oe-lkp/202406141323.413a90d2-lkp@intel.com |
| Fixes: a7f3813e589f ("usb: gadget: dummy_hcd: Switch to hrtimer transfer scheduler") |
| Cc: stable@vger.kernel.org |
| Acked-by: Marcello Sylvester Bauer <sylv@sylv.io> |
| Signed-off-by: Andrey Konovalov <andreyknvl@gmail.com> |
| Reported-by: syzbot+edd9fe0d3a65b14588d5@syzkaller.appspotmail.com |
| Closes: https://syzkaller.appspot.com/bug?extid=edd9fe0d3a65b14588d5 |
| Link: https://lore.kernel.org/r/20240904013051.4409-1-andrey.konovalov@linux.dev |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| drivers/usb/gadget/udc/dummy_hcd.c | 14 ++++++++------ |
| 1 file changed, 8 insertions(+), 6 deletions(-) |
| |
| --- a/drivers/usb/gadget/udc/dummy_hcd.c |
| +++ b/drivers/usb/gadget/udc/dummy_hcd.c |
| @@ -1304,7 +1304,8 @@ static int dummy_urb_enqueue( |
| |
| /* kick the scheduler, it'll do the rest */ |
| if (!hrtimer_active(&dum_hcd->timer)) |
| - hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), HRTIMER_MODE_REL); |
| + hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), |
| + HRTIMER_MODE_REL_SOFT); |
| |
| done: |
| spin_unlock_irqrestore(&dum_hcd->dum->lock, flags); |
| @@ -1325,7 +1326,7 @@ static int dummy_urb_dequeue(struct usb_ |
| rc = usb_hcd_check_unlink_urb(hcd, urb, status); |
| if (!rc && dum_hcd->rh_state != DUMMY_RH_RUNNING && |
| !list_empty(&dum_hcd->urbp_list)) |
| - hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL); |
| + hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL_SOFT); |
| |
| spin_unlock_irqrestore(&dum_hcd->dum->lock, flags); |
| return rc; |
| @@ -1995,7 +1996,8 @@ return_urb: |
| dum_hcd->udev = NULL; |
| } else if (dum_hcd->rh_state == DUMMY_RH_RUNNING) { |
| /* want a 1 msec delay here */ |
| - hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), HRTIMER_MODE_REL); |
| + hrtimer_start(&dum_hcd->timer, ns_to_ktime(DUMMY_TIMER_INT_NSECS), |
| + HRTIMER_MODE_REL_SOFT); |
| } |
| |
| spin_unlock_irqrestore(&dum->lock, flags); |
| @@ -2389,7 +2391,7 @@ static int dummy_bus_resume(struct usb_h |
| dum_hcd->rh_state = DUMMY_RH_RUNNING; |
| set_link_state(dum_hcd); |
| if (!list_empty(&dum_hcd->urbp_list)) |
| - hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL); |
| + hrtimer_start(&dum_hcd->timer, ns_to_ktime(0), HRTIMER_MODE_REL_SOFT); |
| hcd->state = HC_STATE_RUNNING; |
| } |
| spin_unlock_irq(&dum_hcd->dum->lock); |
| @@ -2467,7 +2469,7 @@ static DEVICE_ATTR_RO(urbs); |
| |
| static int dummy_start_ss(struct dummy_hcd *dum_hcd) |
| { |
| - hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); |
| + hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT); |
| dum_hcd->timer.function = dummy_timer; |
| dum_hcd->rh_state = DUMMY_RH_RUNNING; |
| dum_hcd->stream_en_ep = 0; |
| @@ -2497,7 +2499,7 @@ static int dummy_start(struct usb_hcd *h |
| return dummy_start_ss(dum_hcd); |
| |
| spin_lock_init(&dum_hcd->dum->lock); |
| - hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); |
| + hrtimer_init(&dum_hcd->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT); |
| dum_hcd->timer.function = dummy_timer; |
| dum_hcd->rh_state = DUMMY_RH_RUNNING; |
| |