Re: [PATCH] tty: serial: max3100: drain async producers in remove() to fix timer UAF
From: Fan Wu
Date: Thu Jul 30 2026 - 22:48:13 EST
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 <gregkh@xxxxxxxxxxxxxxxxxxx> 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