[Bug 246114] net.isr.dispatch can not be set via loader

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=246114

            Bug ID: 246114
           Summary: net.isr.dispatch can not be set via loader
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Many People
          Priority: ---
         Component: kern
          Assignee: bugs at FreeBSD.org
          Reporter: delphij at FreeBSD.org

In r357614, sysctl_handle_string was refactored to protect against concurrent
writes to sysctl node, and as a result a temporary buffer is allocated via
malloc(9).

Unfortunately, netisr is initialized earlier than malloc(9), as the result,
setting net.isr.dispatch="deferred" in /boot/loader.conf would cause the kernel
to panic immediately:

KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x47/frame 0xffffffff8228d560
vpanic() at vpanic+0x1c7/frame 0xffffffff8228d5c0
panic() at panic+0x43/frame 0xffffffff8228d620
trap_fatal() at trap_fatal+0x4ca/frame 0xffffffff8228d6a0
trap_pfault() at trap_pfault+0xdc/frame 0xffffffff8228d720
trap() at trap+0x3f8/frame 0xffffffff8228d840
calltrap() at calltrap+0x8/frame 0xffffffff8228d840
--- trap 0xc, rip = 0xffffffff8081a985, rsp = 0xffffffff8228d910, rbp =
0xffffffff8228d960 ---
malloc() at malloc+0x95/frame 0xffffffff8228d960
sysctl_handle_string() at sysctl_handle_string+0x215/frame 0xffffffff8228d9a0
sysctl_netisr_dispatch_policy() at sysctl_netisr_dispatch_policy+0x86/frame
0xffffffff8228d9f0
sysctl_root_handler_locked() at sysctl_root_handler_locked+0xf9/frame
0xffffffff8228da40
sysctl_register_oid() at sysctl_register_oid+0x9d1/frame 0xffffffff8228dd60
sysctl_register_all() at sysctl_register_all+0x89/frame 0xffffffff8228dd80
mi_startup() at mi_startup+0x3ac/frame 0xffffffff8228ddf0
btext() at btext+0x2c
KDB: enter: panic
[ thread pid 0 tid 0 ]
Stopped at      kdb_enter+0x67: movq    $0,0xa926e6(%rip)
db>     

A very dirty (and wrong) hack would be to have a short string allocated in
stack buffer and fall back to malloc() only when the string is long, but I
don't like the approach as it's only papering out the actual issue.

svn diff sys/kern/kern_sysctl.c 
Index: sys/kern/kern_sysctl.c
===================================================================
--- sys/kern/kern_sysctl.c      (revision 360560)
+++ sys/kern/kern_sysctl.c      (working copy)
@@ -1646,6 +1646,7 @@ sysctl_handle_string(SYSCTL_HANDLER_ARGS)
        char *tmparg;
        size_t outlen;
        int error = 0, ro_string = 0;
+       char shortbuf[256];

        /*
         * If the sysctl isn't writable and isn't a preallocated tunable that
@@ -1666,7 +1667,11 @@ sysctl_handle_string(SYSCTL_HANDLER_ARGS)
                        tmparg = arg1;
                        outlen = strlen(tmparg) + 1;
                } else {
-                       tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK);
+                       if (arg2 <= sizeof(shortbuf)) {
+                               tmparg = (char *)&shortbuf;
+                       } else {
+                               tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK);
+                       }
                        sx_slock(&sysctlstringlock);
                        memcpy(tmparg, arg1, arg2);
                        sx_sunlock(&sysctlstringlock);
@@ -1675,7 +1680,7 @@ sysctl_handle_string(SYSCTL_HANDLER_ARGS)

                error = SYSCTL_OUT(req, tmparg, outlen);

-               if (!ro_string)
+               if (!ro_string && arg2 > sizeof(shortbuf))
                        free(tmparg, M_SYSCTLTMP);
        } else {
                if (!ro_string)
@@ -1697,11 +1702,16 @@ sysctl_handle_string(SYSCTL_HANDLER_ARGS)
                sx_xunlock(&sysctlstringlock);
        } else {
                arg2 = req->newlen - req->newidx;
-               tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK);
+               if (arg2 <= sizeof(shortbuf)) {
+                       tmparg = (char *)&shortbuf;
+               } else {
+                       tmparg = malloc(arg2, M_SYSCTLTMP, M_WAITOK);
+               }

                error = SYSCTL_IN(req, tmparg, arg2);
                if (error) {
-                       free(tmparg, M_SYSCTLTMP);
+                       if (arg2 > sizeof(shortbuf))
+                               free(tmparg, M_SYSCTLTMP);
                        return (error);
                }

@@ -1709,7 +1719,8 @@ sysctl_handle_string(SYSCTL_HANDLER_ARGS)
                memcpy(arg1, tmparg, arg2);
                ((char *)arg1)[arg2] = '\0';
                sx_xunlock(&sysctlstringlock);
-               free(tmparg, M_SYSCTLTMP);
+               if (arg2 > sizeof(shortbuf))
+                       free(tmparg, M_SYSCTLTMP);
                req->newidx += arg2;
        }
        return (error);

-- 
You are receiving this mail because:
You are the assignee for the bug.