Re: [PATCH v2] can: isotp: handle wait_event_interruptible() return values

[Oliver Hartkopp <socketcan@hartkopp.net>]

I sent a V4 (recommended) so that you can pick V3/V4 at your will (see 
explanation below)

Thanks,
Oliver

ps. open patches:

[PATCH can-next] can: isotp: check CAN address family in isotp_bind()
https://lore.kernel.org/linux-can/20230104201844.13168-1-socketcan@hartkopp.net/T/#u

The below patch is needed to silence the Syzcaller
https://syzkaller.appspot.com/bug?extid=5aed6c3aaba661f5b917

[PATCH] can: isotp: split tx timer into transmission and timeout
https://lore.kernel.org/linux-can/20230104145701.2422-1-socketcan@hartkopp.net/T/#u

And the V4 patch.

https://lore.kernel.org/linux-can/20230112192347.1944-1-socketcan@hartkopp.net/T/#u

On 06.01.23 13:59, Oliver Hartkopp wrote:
> 
> 
> On 06.01.23 12:37, Marc Kleine-Budde wrote:
>> On 05.01.2023 13:58:30, Oliver Hartkopp wrote:
>>>
>>>
>>> On 05.01.23 10:32, Marc Kleine-Budde wrote:
>>>> On 04.01.2023 17:46:05, Oliver Hartkopp wrote:
>>>>> When wait_event_interruptible() has been interrupted by a signal the
>>>>> tx.state value might not be ISOTP_IDLE. Force the state machines
>>>>> into idle state to inhibit the timer handlers to continue working.
>>>>>
>>>>> Cc: stable@vger.kernel.org # >= v5.15
>>>>> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
>>>>
>>>> Can you add a Fixes: tag?
>>>
>>> Yes. Sent out a V3.
>>>
>>> In fact I was not sure if it makes sense to apply the patch down to 
>>> Linux
>>> 5.10 as it might increase the possibility to trigger a WARN(1) in the
>>> isotp_tx_timer_handler().
>>>
>>> The patch is definitely helpful for the latest code including commit
>>> 4b7fe92c0690 ("can: isotp: add local echo tx processing for consecutive
>>> frames") introduced in Linux 5.18 and its fixes.
>>>
>>> I did some testing with very long ISOTP PDUs and killed the waiting
>>> isotp_release() with a Crtl-C.
>>>
>>> To prevent the WARN(1) we might also stick this patch to
>>>
>>> Fixes: 866337865f37 ("can: isotp: fix tx state handling for echo tx
>>> processing")
>>>
>>> What do you think about the WARN(1)?
>>
>> If this short patch avoids potential WARN()s it's stable material.
>>
> 
> It is the other way around. As written above this patch might increase 
> the possibility to trigger a WARN(1).
> 
> With the patch:
> 
> https://lore.kernel.org/linux-can/20230104145701.2422-1-socketcan@hartkopp.net/T/#u
> 
> the WARN(1) is removed and this patch here makes the situation better.
> 
> Alternatively I could provide another patch for kernels < v6.0 which set 
> the rx/tx states AND remove the WARN(1).
> 
> Back to your original question about the "Fixes:" tag, I would suggest 
> to tag this patch similar to
> 
> https://lore.kernel.org/linux-can/20230104145701.2422-1-socketcan@hartkopp.net/T/#u
> 
> Namely:
> 
> Fixes: 866337865f37 ("can: isotp: fix tx state handling for echo tx 
> processing")
> Cc: stable@vger.kernel.org # >= v6.0
> 
> And later check whether I should remove the WARN(1) in older stable 
> kernels.
> 
> Should I send a V4 with the new "Fixes: 866337865f37" tag?
> 
> Best regards,
> Oliver
>