netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] can: isotp: add result check for wait_event_interruptible()
@ 2021-09-18  9:28 Ziyang Xuan
  2021-09-27  1:39 ` Ziyang Xuan (William)
  0 siblings, 1 reply; 3+ messages in thread
From: Ziyang Xuan @ 2021-09-18  9:28 UTC (permalink / raw)
  To: socketcan; +Cc: mkl, davem, kuba, linux-can, netdev

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>
---
 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)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] can: isotp: add result check for wait_event_interruptible()
  2021-09-18  9:28 [PATCH net] can: isotp: add result check for wait_event_interruptible() Ziyang Xuan
@ 2021-09-27  1:39 ` Ziyang Xuan (William)
  2021-09-27 11:32   ` Oliver Hartkopp
  0 siblings, 1 reply; 3+ messages in thread
From: Ziyang Xuan (William) @ 2021-09-27  1:39 UTC (permalink / raw)
  To: socketcan; +Cc: mkl, davem, kuba, linux-can, netdev

> 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>

Hi Oliver,
I send a new patch with this problem, ignore this patch please.

Thank you!

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] can: isotp: add result check for wait_event_interruptible()
  2021-09-27  1:39 ` Ziyang Xuan (William)
@ 2021-09-27 11:32   ` Oliver Hartkopp
  0 siblings, 0 replies; 3+ messages in thread
From: Oliver Hartkopp @ 2021-09-27 11:32 UTC (permalink / raw)
  To: Ziyang Xuan (William); +Cc: mkl, davem, kuba, linux-can, netdev



On 27.09.21 03:39, Ziyang Xuan (William) 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>
> 
> Hi Oliver,
> I send a new patch with this problem, ignore this patch please.
> 
> Thank you!
> 

I was still pondering on this patch ;-)

But I'll take a look on the update now.

Thanks & best regards,
Oliver

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-27 11:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-18  9:28 [PATCH net] can: isotp: add result check for wait_event_interruptible() Ziyang Xuan
2021-09-27  1:39 ` Ziyang Xuan (William)
2021-09-27 11:32   ` Oliver Hartkopp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).