Re: [PATCH] tty: serial: max3100: drain async producers in remove() to fix timer UAF
Fan Wu <[email protected]> Fri, 31 Jul 2026 10:46:31 +0800
| Newsgroups | gmane.linux.kernel,gmane.linux.serial,gmane.linux.kernel.stable |
|---|---|
| Message-ID | <[email protected]> |
Hi Greg, Thanks for the review. You're right. I'll respin with a concise changelog, remove the boolean helper, and rename the state to irq_registered. I'll send v2 shortly. Thanks, Fan > On Jul 30, 2026, at 22:39, Greg KH <[email protected]> wrote: > > Yes, but that's not needed here, right? > > LLMs love to write text, please don't let it and write your own > changelog that actually makes sense :) > >> >> +/* >> + * Stop async producers before tearing down the workqueue. A normal >> + * shutdown must leave the timer re-armable for the next open, while final >> + * removal uses timer_shutdown_sync() to prevent max3100_timeout() from >> + * re-arming a timer embedded in an object about to be freed. >> + */ >> +static void max3100_drain_async(struct max3100_port *s, bool final) > > Having a bool here makes no sense at all, and it's a horrible api as now > you have to look up the documentation each time you run across it. > > Also, you didn't even document it (well, the LLM didn't...) > > Please be more careful. > >> +{ >> + s->force_end_work = 1; >> + if (final) >> + timer_shutdown_sync(&s->timer); >> + else >> + timer_delete_sync(&s->timer); >> + if (s->irq_requested) { >> + free_irq(s->port.irq, s); >> + s->irq_requested = false; > > It's not "requested" it's "is an interrupt registered or not", right? > > thanks, > > greg k-h