can: isotp: split tx timer into transmission and timeout

The timer for the transmission of isotp PDUs formerly had two functions:
1. send two consecutive frames with a given time gap
2. monitor the timeouts for flow control frames and the echo frames

This led to larger txstate checks and potentially to a problem discovered
by syzbot which enabled the panic_on_warn feature while testing.

The former 'txtimer' function is split into 'txfrtimer' and 'txtimer'
to handle the two above functionalities with separate timer callbacks.

The two simplified timers now run in one-shot mode and make the state
transitions (especially with isotp_rcv_echo) better understandable.

Fixes: 866337865f ("can: isotp: fix tx state handling for echo tx processing")
Reported-by: syzbot+5aed6c3aaba661f5b917@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org # >= v6.0
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Link: https://lore.kernel.org/all/20230104145701.2422-1-socketcan@hartkopp.net
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This commit is contained in:
Oliver Hartkopp 2023-01-04 15:57:01 +01:00 committed by Marc Kleine-Budde
parent 823b2e4272
commit 4f027cba82

View File

@ -140,7 +140,7 @@ struct isotp_sock {
canid_t rxid;
ktime_t tx_gap;
ktime_t lastrxcf_tstamp;
struct hrtimer rxtimer, txtimer;
struct hrtimer rxtimer, txtimer, txfrtimer;
struct can_isotp_options opt;
struct can_isotp_fc_options rxfc, txfc;
struct can_isotp_ll_options ll;
@ -871,7 +871,7 @@ static void isotp_rcv_echo(struct sk_buff *skb, void *data)
}
/* start timer to send next consecutive frame with correct delay */
hrtimer_start(&so->txtimer, so->tx_gap, HRTIMER_MODE_REL_SOFT);
hrtimer_start(&so->txfrtimer, so->tx_gap, HRTIMER_MODE_REL_SOFT);
}
static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
@ -879,49 +879,39 @@ static enum hrtimer_restart isotp_tx_timer_handler(struct hrtimer *hrtimer)
struct isotp_sock *so = container_of(hrtimer, struct isotp_sock,
txtimer);
struct sock *sk = &so->sk;
enum hrtimer_restart restart = HRTIMER_NORESTART;
switch (so->tx.state) {
case ISOTP_SENDING:
/* don't handle timeouts in IDLE state */
if (so->tx.state == ISOTP_IDLE)
return HRTIMER_NORESTART;
/* cfecho should be consumed by isotp_rcv_echo() here */
if (!so->cfecho) {
/* start timeout for unlikely lost echo skb */
hrtimer_set_expires(&so->txtimer,
ktime_add(ktime_get(),
ktime_set(ISOTP_ECHO_TIMEOUT, 0)));
restart = HRTIMER_RESTART;
/* we did not get any flow control or echo frame in time */
/* push out the next consecutive frame */
isotp_send_cframe(so);
break;
}
/* report 'communication error on send' */
sk->sk_err = ECOMM;
if (!sock_flag(sk, SOCK_DEAD))
sk_error_report(sk);
/* cfecho has not been cleared in isotp_rcv_echo() */
pr_notice_once("can-isotp: cfecho %08X timeout\n", so->cfecho);
fallthrough;
/* reset tx state */
so->tx.state = ISOTP_IDLE;
wake_up_interruptible(&so->wait);
case ISOTP_WAIT_FC:
case ISOTP_WAIT_FIRST_FC:
return HRTIMER_NORESTART;
}
/* we did not get any flow control frame in time */
static enum hrtimer_restart isotp_txfr_timer_handler(struct hrtimer *hrtimer)
{
struct isotp_sock *so = container_of(hrtimer, struct isotp_sock,
txfrtimer);
/* report 'communication error on send' */
sk->sk_err = ECOMM;
if (!sock_flag(sk, SOCK_DEAD))
sk_error_report(sk);
/* start echo timeout handling and cover below protocol error */
hrtimer_start(&so->txtimer, ktime_set(ISOTP_ECHO_TIMEOUT, 0),
HRTIMER_MODE_REL_SOFT);
/* reset tx state */
so->tx.state = ISOTP_IDLE;
wake_up_interruptible(&so->wait);
break;
/* cfecho should be consumed by isotp_rcv_echo() here */
if (so->tx.state == ISOTP_SENDING && !so->cfecho)
isotp_send_cframe(so);
default:
WARN_ONCE(1, "can-isotp: tx timer state %08X cfecho %08X\n",
so->tx.state, so->cfecho);
}
return restart;
return HRTIMER_NORESTART;
}
static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size)
@ -1198,6 +1188,7 @@ static int isotp_release(struct socket *sock)
}
}
hrtimer_cancel(&so->txfrtimer);
hrtimer_cancel(&so->txtimer);
hrtimer_cancel(&so->rxtimer);
@ -1601,6 +1592,8 @@ static int isotp_init(struct sock *sk)
so->rxtimer.function = isotp_rx_timer_handler;
hrtimer_init(&so->txtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
so->txtimer.function = isotp_tx_timer_handler;
hrtimer_init(&so->txfrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL_SOFT);
so->txfrtimer.function = isotp_txfr_timer_handler;
init_waitqueue_head(&so->wait);
spin_lock_init(&so->rx_lock);