netdev - Re: [PATCH] can: isotp: fix tx state handling for echo tx processing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221104121059.kbhrpwbumuc6q3iv@pengutronix.de>
Date:   Fri, 4 Nov 2022 13:10:59 +0100
From:   Marc Kleine-Budde <mkl@...gutronix.de>
To:     Oliver Hartkopp <socketcan@...tkopp.net>
Cc:     netdev@...r.kernel.org, linux-can@...r.kernel.org,
        Wei Chen <harperchen1110@...il.com>, stable@...r.kernel.org
Subject: Re: [PATCH] can: isotp: fix tx state handling for echo tx processing

On 01.11.2022 22:29:02, Oliver Hartkopp wrote:
> In commit 4b7fe92c0690 ("can: isotp: add local echo tx processing for
> consecutive frames") the data flow for consecutive frames (CF) has been
> reworked to improve the reliability of long data transfers.
> 
> This rework did not touch the transmission and the tx state changes of
> single frame (SF) transfers which likely led to the WARN in the
> isotp_tx_timer_handler() catching a wrong tx state. This patch makes use
> of the improved frame processing for SF frames and sets the ISOTP_SENDING
> state in isotp_sendmsg() within the cmpxchg() condition handling.
> 
> A review of the state machine and the timer handling additionally revealed
> a missing echo timeout handling in the case of the burst mode in
> isotp_rcv_echo() and removes a potential timer configuration uncertainty
> in isotp_rcv_fc() when the receiver requests consecutive frames.
> 
> Fixes: 4b7fe92c0690 ("can: isotp: add local echo tx processing for consecutive frames")
> Link: https://lore.kernel.org/linux-can/CAO4mrfe3dG7cMP1V5FLUkw7s+50c9vichigUMQwsxX4M=45QEw@mail.gmail.com/T/#u
> Reported-by: Wei Chen <harperchen1110@...il.com>
> Cc: stable@...r.kernel.org # v6.0
> Signed-off-by: Oliver Hartkopp <socketcan@...tkopp.net>

[...]

> @@ -905,10 +915,11 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
>  		so->tx.state = ISOTP_IDLE;
>  		wake_up_interruptible(&so->wait);
>  		break;
>  
>  	default:
> +		pr_notice_once("can-isotp: tx timer state %X\n", so->tx.state);
>  		WARN_ON_ONCE(1);

Can you use WARN_ONCE() instead of pr_notice_once() + WARN_ON_ONCE() here?

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)