Messages in this thread | | | Subject | Re: [PATCH net v2 1/2] can: isotp: add result check for wait_event_interruptible() | From | Oliver Hartkopp <> | Date | Mon, 11 Oct 2021 21:10:46 +0200 |
| |
On 09.10.21 09:40, Ziyang Xuan wrote: > Using wait_event_interruptible() to wait for complete transmission, > but do not check the result of wait_event_interruptible() which can > be interrupted. It will result in tx buffer has multiple accessers > and the later process interferes with the previous process. > > Following is one of the problems reported by syzbot. > > ============================================================= > WARNING: CPU: 0 PID: 0 at net/can/isotp.c:840 isotp_tx_timer_handler+0x2e0/0x4c0 > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.13.0-rc7+ #68 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/01/2014 > RIP: 0010:isotp_tx_timer_handler+0x2e0/0x4c0 > Call Trace: > <IRQ> > ? isotp_setsockopt+0x390/0x390 > __hrtimer_run_queues+0xb8/0x610 > hrtimer_run_softirq+0x91/0xd0 > ? rcu_read_lock_sched_held+0x4d/0x80 > __do_softirq+0xe8/0x553 > irq_exit_rcu+0xf8/0x100 > sysvec_apic_timer_interrupt+0x9e/0xc0 > </IRQ> > asm_sysvec_apic_timer_interrupt+0x12/0x20 > > Add result check for wait_event_interruptible() in isotp_sendmsg() > to avoid multiple accessers for tx buffer. > > Reported-by: syzbot+78bab6958a614b0c80b9@syzkaller.appspotmail.com > Fixes: e057dd3fc20f ("can: add ISO 15765-2:2016 transport protocol") > Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Many thanks! Oliver
> --- > net/can/isotp.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/can/isotp.c b/net/can/isotp.c > index caaa532ece94..2ac29c2b2ca6 100644 > --- a/net/can/isotp.c > +++ b/net/can/isotp.c > @@ -865,7 +865,9 @@ static int isotp_sendmsg(struct socket *sock, struct msghdr *msg, size_t size) > return -EAGAIN; > > /* wait for complete transmission of current pdu */ > - wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); > + err = wait_event_interruptible(so->wait, so->tx.state == ISOTP_IDLE); > + if (err) > + return err; > } > > if (!size || size > MAX_MSG_LENGTH) >
| |