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 org.kernel.vger.linux-serial,org.kernel.vger.linux-kernel,org.kernel.vger.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