| Message ID | 20220425115701.2197-1-socketcan@hartkopp.net |
|---|---|
| State | New |
| Headers | show |
| Series | [5.10.y] can: isotp: stop timeout monitoring when no first frame was sent | expand |
On Mon, Apr 25, 2022 at 01:57:01PM +0200, Oliver Hartkopp wrote: > [ Upstream commit d73497081710c876c3c61444445512989e102152 ] > > The first attempt to fix a the 'impossible' WARN_ON_ONCE(1) in > isotp_tx_timer_handler() focussed on the identical CAN IDs created by > the syzbot reproducer and lead to upstream fix/commit 3ea566422cbd > ("can: isotp: sanitize CAN ID checks in isotp_bind()"). But this did > not catch the root cause of the wrong tx.state in the tx_timer handler. > > In the isotp 'first frame' case a timeout monitoring needs to be started > before the 'first frame' is send. But when this sending failed the timeout > monitoring for this specific frame has to be disabled too. > > Otherwise the tx_timer is fired with the 'warn me' tx.state of ISOTP_IDLE. > > Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") > Link: https://lore.kernel.org/all/20220405175112.2682-1-socketcan@hartkopp.net > Reported-by: syzbot+2339c27f5c66c652843e@syzkaller.appspotmail.com > Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > net/can/isotp.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) Now queued up, thanks. greg k-h
diff --git a/net/can/isotp.c b/net/can/isotp.c index 9a4a9c5a9f24..c515bbd46c67 100644 --- a/net/can/isotp.c +++ b/net/can/isotp.c @@ -862,10 +862,11 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) struct sk_buff *skb; struct net_device *dev; struct canfd_frame *cf; int ae = (so->opt.flags & CAN_ISOTP_EXTEND_ADDR) ? 1 : 0; int wait_tx_done = (so->opt.flags & CAN_ISOTP_WAIT_TX_DONE) ? 1 : 0; + s64 hrtimer_sec = 0; int off; int err; if (!so->bound) return -EADDRNOTAVAIL; @@ -960,11 +961,13 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) /* send first frame and wait for FC */ isotp_create_fframe(cf, so, ae); /* start timeout for FC */ - hrtimer_start(&so->txtimer, ktime_set(1, 0), HRTIMER_MODE_REL_SOFT); + hrtimer_sec = 1; + hrtimer_start(&so->txtimer, ktime_set(hrtimer_sec, 0), + HRTIMER_MODE_REL_SOFT); } /* send the first or only CAN frame */ cf->flags = so->ll.tx_flags; @@ -973,10 +976,15 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) err = can_send(skb, 1); dev_put(dev); if (err) { pr_notice_once("can-isotp: %s: can_send_ret %d\n", __func__, err); + + /* no transmission -> no timeout monitoring */ + if (hrtimer_sec) + hrtimer_cancel(&so->txtimer); + goto err_out_drop; } if (wait_tx_done) { /* wait for complete transmission of current pdu */
[ Upstream commit d73497081710c876c3c61444445512989e102152 ] The first attempt to fix a the 'impossible' WARN_ON_ONCE(1) in isotp_tx_timer_handler() focussed on the identical CAN IDs created by the syzbot reproducer and lead to upstream fix/commit 3ea566422cbd ("can: isotp: sanitize CAN ID checks in isotp_bind()"). But this did not catch the root cause of the wrong tx.state in the tx_timer handler. In the isotp 'first frame' case a timeout monitoring needs to be started before the 'first frame' is send. But when this sending failed the timeout monitoring for this specific frame has to be disabled too. Otherwise the tx_timer is fired with the 'warn me' tx.state of ISOTP_IDLE. Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") Link: https://lore.kernel.org/all/20220405175112.2682-1-socketcan@hartkopp.net Reported-by: syzbot+2339c27f5c66c652843e@syzkaller.appspotmail.com Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> --- net/can/isotp.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)