Re: [PATCH v2] mmc: vub300: fix sleeping function called from invalid context
Johan Hovold <[email protected]>
| Newsgroups | org.kernel.vger.linux-mmc,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Tue, Aug 18, 2026 at 05:08:43PM +0200, Johan Hovold wrote: > On Tue, Aug 18, 2026 at 05:36:56PM +0300, Ömer Mete Kaya wrote: > > syzbot reports: > > > > BUG: sleeping function called from invalid context at kernel/workqueue.c:4487 > > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 0, name: swapper/1 > > ... > > <IRQ> > > __might_resched > > __cancel_work_sync > > mmc_free_host+0x19/0x30 [drivers/mmc/core/host.c:700] > > call_timer_fn+0x192/0x5e0 [kernel/time/timer.c:1748] > > run_timer_softirq > > ... > > > > vub300_inactivity_timer_expired() runs in softirq (timer) context. > > When the USB interface had already gone away (->interface == NULL, > > cleared by vub300_disconnect() or the probe() error path), the timer > > handler dropped the object's last kref via > > kref_put(&vub300->kref, vub300_delete). If that was the last > > reference, vub300_delete() ran from softirq context and called > > mmc_free_host(), which calls cancel_delayed_work_sync() - a sleeping > > function, illegal from softirq/timer context. > > > > Root cause: inactivity_timer is armed in probe() and continuously > > re-armed via mod_timer(), but - unlike sg_transfer_timer, which is > > explicitly deleted after each use - it is never stopped when the > > device is torn down, so it can still fire after ->interface has > > been cleared. > > > > Fix this by decoupling inactivity_timer from the object's kref > > entirely: drop the kref_get() taken on its behalf in probe(); make > > vub300_inactivity_timer_expired() a no-op when ->interface is NULL > > instead of dropping a reference; and in both vub300_disconnect() and > > the probe() err_stop_io path, call > > timer_delete_sync(&vub300->inactivity_timer) right after clearing > > ->interface and before the final kref_put(). Since ->interface is > > already NULL at that point, any concurrently running timer instance > > takes the no-op branch, so timer_delete_sync() is guaranteed to > > return with the timer stopped for good - removing any race with the > > final kref_put()/vub300_delete()/mmc_free_host(). Before this > > patch, a successful probe() left two references on the kref (one > > from kref_init(), one from the timer's kref_get()); after it, only > > the initial kref_init() reference remains, matching the single > > kref_put() in vub300_disconnect() and err_stop_io. > > > > While auditing the driver for the same class of bug, also switch > > sg_transfer_timer's two timer_delete() call sites (in > > __command_read_data() and __command_write_data()) to > > timer_delete_sync(), since usb_sg_wait() returning does not > > guarantee a concurrently running vub300_sg_timed_out() has finished. > > __command_write_data() additionally only deleted the timer on the > > success path, leaking an armed timer on the cmd->error path; the > > (now synchronous) delete is moved before that check so it always > > runs. > > > > Reported-by: [email protected] > > Closes: https://syzkaller.appspot.com/bug?extid=0e06aa1bdc6495bac24b > > Fixes: 88095e7b473a ("mmc: Add new VUB300 USB-to-SD/SDIO/MMC driver") > > This is not the commit that introduced the issue. See my reply to [2]. > > > Assisted-by: Claude:claude-sonnet-4-6 > > Signed-off-by: Ömer Mete Kaya <[email protected]> > > --- > > v2: I used an LLM to help structure and polish the English commit > > message. I did not know > > this required an Assisted-by tag until Johan pointed it out - added > > now. > > So you didn't use an LLM to write the proposed fix itself? The commit > message and comments in the code makes it look that way. > > Also note that the syzbot LLM created a similar fix the day before you > posted yours: > > [1] https://lore.kernel.org/all/[email protected]/ Sorry, that was supposed to say: [1] https://lore.kernel.org/all/[email protected]/ Apparently the bot produced two different fixes for the same issue... > and there are at least two further proposals: > > [2] https://lore.kernel.org/all/[email protected]/ > [3] https://lore.kernel.org/all/[email protected]/ Johan