[PATCH net] mISDN: Fix data race in timer handling

Henry Zhang posted 1 patch 1 week, 3 days ago
drivers/isdn/mISDN/timerdev.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH net] mISDN: Fix data race in timer handling
Posted by Henry Zhang 1 week, 3 days ago
KCSAN reported a data race where misdn_add_timer() writes dev->work
while mISDN_read() reads it without holding the spinlock:

write to 0xffff88812d848280 of 4 bytes by task 10864 on cpu 1:
 misdn_add_timer drivers/isdn/mISDN/timerdev.c:175 [inline]
...

read to 0xffff88812d848280 of 4 bytes by task 10857 on cpu 0:
 mISDN_read+0x1f2/0x470 drivers/isdn/mISDN/timerdev.c:112
...

dev->work is read locklessly in wait_event_interruptible() and
mISDN_poll(). In mISDN_read(), the result is rechecked under dev->lock.
In mISDN_poll(), a stale value may cause a spurious EPOLLIN or a missed
wake, but wake_up_interruptible() will correct this. In both cases, the
race is benign, so we can annotate these with READ_ONCE/WRITE_ONCE.

Reported-by: syzbot+c6e7bcea7ffb7ff46acb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=c6e7bcea7ffb7ff46acb

Signed-off-by: Henry Zhang <zeri@umich.edu>
---
 drivers/isdn/mISDN/timerdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/isdn/mISDN/timerdev.c b/drivers/isdn/mISDN/timerdev.c
index df98144a9539..f98f2e0cfb9f 100644
--- a/drivers/isdn/mISDN/timerdev.c
+++ b/drivers/isdn/mISDN/timerdev.c
@@ -109,7 +109,7 @@ mISDN_read(struct file *filep, char __user *buf, size_t count, loff_t *off)
 		spin_unlock_irq(&dev->lock);
 		if (filep->f_flags & O_NONBLOCK)
 			return -EAGAIN;
-		wait_event_interruptible(dev->wait, (dev->work ||
+		wait_event_interruptible(dev->wait, (READ_ONCE(dev->work) ||
 						     !list_empty(list)));
 		if (signal_pending(current))
 			return -ERESTARTSYS;
@@ -143,11 +143,11 @@ mISDN_poll(struct file *filep, poll_table *wait)
 	if (dev) {
 		poll_wait(filep, &dev->wait, wait);
 		mask = 0;
-		if (dev->work || !list_empty(&dev->expired))
+		if (READ_ONCE(dev->work) || !list_empty(&dev->expired))
 			mask |= (EPOLLIN | EPOLLRDNORM);
 		if (*debug & DEBUG_TIMER)
 			printk(KERN_DEBUG "%s work(%d) empty(%d)\n", __func__,
-			       dev->work, list_empty(&dev->expired));
+			       READ_ONCE(dev->work), list_empty(&dev->expired));
 	}
 	return mask;
 }
@@ -172,7 +172,7 @@ misdn_add_timer(struct mISDNtimerdev *dev, int timeout)
 	struct mISDNtimer	*timer;
 
 	if (!timeout) {
-		dev->work = 1;
+		WRITE_ONCE(dev->work, 1);
 		wake_up_interruptible(&dev->wait);
 		id = 0;
 	} else {
-- 
2.34.1
Re: [PATCH net] mISDN: Fix data race in timer handling
Posted by Simon Horman 1 week, 2 days ago
+ Eric

On Tue, Jan 27, 2026 at 02:19:27AM -0500, Henry Zhang wrote:
> KCSAN reported a data race where misdn_add_timer() writes dev->work
> while mISDN_read() reads it without holding the spinlock:
> 
> write to 0xffff88812d848280 of 4 bytes by task 10864 on cpu 1:
>  misdn_add_timer drivers/isdn/mISDN/timerdev.c:175 [inline]
> ...
> 
> read to 0xffff88812d848280 of 4 bytes by task 10857 on cpu 0:
>  mISDN_read+0x1f2/0x470 drivers/isdn/mISDN/timerdev.c:112
> ...
> 
> dev->work is read locklessly in wait_event_interruptible() and
> mISDN_poll(). In mISDN_read(), the result is rechecked under dev->lock.
> In mISDN_poll(), a stale value may cause a spurious EPOLLIN or a missed
> wake, but wake_up_interruptible() will correct this. In both cases, the
> race is benign, so we can annotate these with READ_ONCE/WRITE_ONCE.
> 
> Reported-by: syzbot+c6e7bcea7ffb7ff46acb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=c6e7bcea7ffb7ff46acb
> 
> Signed-off-by: Henry Zhang <zeri@umich.edu>

Hi Henry,

This appears to already be addressed by
commit 8175dbf174d4 ("mISDN: annotate data-race around dev->work")

-- 
pw-bot: cr
Re: [PATCH net] mISDN: Fix data race in timer handling
Posted by Henry Zhang 1 week, 2 days ago
On Wed, Jan 28, 2026 at 12:37 PM Simon Horman <horms@kernel.org> wrote:
>
> + Eric
>
> On Tue, Jan 27, 2026 at 02:19:27AM -0500, Henry Zhang wrote:
> > KCSAN reported a data race where misdn_add_timer() writes dev->work
> > while mISDN_read() reads it without holding the spinlock:
> >
> > write to 0xffff88812d848280 of 4 bytes by task 10864 on cpu 1:
> >  misdn_add_timer drivers/isdn/mISDN/timerdev.c:175 [inline]
> > ...
> >
> > read to 0xffff88812d848280 of 4 bytes by task 10857 on cpu 0:
> >  mISDN_read+0x1f2/0x470 drivers/isdn/mISDN/timerdev.c:112
> > ...
> >
> > dev->work is read locklessly in wait_event_interruptible() and
> > mISDN_poll(). In mISDN_read(), the result is rechecked under dev->lock.
> > In mISDN_poll(), a stale value may cause a spurious EPOLLIN or a missed
> > wake, but wake_up_interruptible() will correct this. In both cases, the
> > race is benign, so we can annotate these with READ_ONCE/WRITE_ONCE.
> >
> > Reported-by: syzbot+c6e7bcea7ffb7ff46acb@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=c6e7bcea7ffb7ff46acb
> >
> > Signed-off-by: Henry Zhang <zeri@umich.edu>
>
> Hi Henry,
>
> This appears to already be addressed by
> commit 8175dbf174d4 ("mISDN: annotate data-race around dev->work")
>
> --
> pw-bot: cr
Hi Simon,

Thanks for pointing this out. Dropping this patch.

Henry