Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/usb usb(4): Use atomics for usb_async_proc.



details:   https://anonhg.NetBSD.org/src/rev/eccb05c0cea5
branches:  trunk
changeset: 363361:eccb05c0cea5
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun Mar 06 09:03:42 2022 +0000

description:
usb(4): Use atomics for usb_async_proc.

This is written under proc_lock and read without it in usb_add_event,
so using atomics pacifies the sanitizer.  No memory ordering needed
because the value isn't actually used until the softint runs, using
it under proc_lock.  Kind of a micro-optimization, but let's avoid
contention on proc_lock in the common case of no usb_async_proc set
up (why is this a system global, anyway? and why is there a softint
if usb_add_event always runs at IPL_NONE?).

Reported-by: syzbot+1b2fa68535e5b0f3dcaa%syzkaller.appspotmail.com@localhost

diffstat:

 sys/dev/usb/usb.c |  18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)

diffs (67 lines):

diff -r 8394f5c567cb -r eccb05c0cea5 sys/dev/usb/usb.c
--- a/sys/dev/usb/usb.c Sun Mar 06 08:31:54 2022 +0000
+++ b/sys/dev/usb/usb.c Sun Mar 06 09:03:42 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: usb.c,v 1.198 2021/10/10 20:14:09 jmcneill Exp $       */
+/*     $NetBSD: usb.c,v 1.199 2022/03/06 09:03:42 riastradh Exp $      */
 
 /*
  * Copyright (c) 1998, 2002, 2008, 2012 The NetBSD Foundation, Inc.
@@ -37,7 +37,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.198 2021/10/10 20:14:09 jmcneill Exp $");
+__KERNEL_RCSID(0, "$NetBSD: usb.c,v 1.199 2022/03/06 09:03:42 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_usb.h"
@@ -833,7 +833,7 @@
                        return EBUSY;
                usb_dev_open = 1;
                mutex_enter(&proc_lock);
-               usb_async_proc = NULL;
+               atomic_store_relaxed(&usb_async_proc, NULL);
                mutex_exit(&proc_lock);
                return 0;
        }
@@ -913,7 +913,7 @@
 
        if (unit == USB_DEV_MINOR) {
                mutex_enter(&proc_lock);
-               usb_async_proc = NULL;
+               atomic_store_relaxed(&usb_async_proc, NULL);
                mutex_exit(&proc_lock);
                usb_dev_open = 0;
        }
@@ -937,10 +937,8 @@
 
                case FIOASYNC:
                        mutex_enter(&proc_lock);
-                       if (*(int *)data)
-                               usb_async_proc = l->l_proc;
-                       else
-                               usb_async_proc = NULL;
+                       atomic_store_relaxed(&usb_async_proc,
+                           *(int *)data ? l->l_proc : NULL);
                        mutex_exit(&proc_lock);
                        return 0;
 
@@ -1317,7 +1315,7 @@
        SIMPLEQ_INSERT_TAIL(&usb_events, ueq, next);
        cv_signal(&usb_event_cv);
        selnotify(&usb_selevent, 0, 0);
-       if (usb_async_proc != NULL) {
+       if (atomic_load_relaxed(&usb_async_proc) != NULL) {
                kpreempt_disable();
                softint_schedule(usb_async_sih);
                kpreempt_enable();
@@ -1331,7 +1329,7 @@
        proc_t *proc;
 
        mutex_enter(&proc_lock);
-       if ((proc = usb_async_proc) != NULL)
+       if ((proc = atomic_load_relaxed(&usb_async_proc)) != NULL)
                psignal(proc, SIGIO);
        mutex_exit(&proc_lock);
 }



Home | Main Index | Thread Index | Old Index