Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 29 Aug 2020 04:04:55 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        Mateusz Guzik <mjguzik@gmail.com>
Cc:        Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r364946 - head/sys/kern
Message-ID:  <CANCZdfqXtKhKhh33ovFQ4_a3tiesRi8-6ZuMTp0yW%2BMzkxWLzA@mail.gmail.com>
In-Reply-To: <CAGudoHFAkrAykin6ngH=04254J4AmhHk2NmDyGfrUE=wJcxH2A@mail.gmail.com>
References:  <202008290430.07T4UCM4007928@repo.freebsd.org> <CAGudoHFAkrAykin6ngH=04254J4AmhHk2NmDyGfrUE=wJcxH2A@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Aug 29, 2020 at 1:09 AM Mateusz Guzik <mjguzik@gmail.com> wrote:

> This crashes on boot for me:
>

I wasn't able to get it to crash on boot for me, but I was able to recreate
it. Fixed in r364949. I think it didn't crash on boot for me because
kldxref failed due to the segment thing so devmatch didn't run which would
have triggered this bug. devinfo did trigger a very similar crash, and
r364949 fixes that crash. Even a new kldxref failed due to the too many
segments thing, so I can't confirm that's what you hit, but I'm pretty sure
it is...

Warner


> atal trap 12: page fault while in kernel mode
> cpuid = 0; apic id = 00
> fault virtual address   = 0x0
> fault code              = supervisor read data, page not present
> instruction pointer     = 0x20:0xffffffff805b0a7f
> stack pointer           = 0x28:0xfffffe002366a7f0
> frame pointer           = 0x28:0xfffffe002366a7f0
> code segment            = base 0x0, limit 0xfffff, type 0x1b
>                         = DPL 0, pres 1, long 1, def32 0, gran 1
> processor eflags        = interrupt enabled, resume, IOPL = 0
> current process         = 89 (devmatch)
> trap number             = 12
> panic: page fault
> cpuid = 0
> time = 1598692135
> KDB: stack backtrace:
> db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame
> 0xfffffe002366a4a0
> vpanic() at vpanic+0x182/frame 0xfffffe002366a4f0
> panic() at panic+0x43/frame 0xfffffe002366a550
> trap_fatal() at trap_fatal+0x387/frame 0xfffffe002366a5b0
> trap_pfault() at trap_pfault+0x4f/frame 0xfffffe002366a610
> trap() at trap+0x27d/frame 0xfffffe002366a720
> calltrap() at calltrap+0x8/frame 0xfffffe002366a720
> --- trap 0xc, rip = 0xffffffff805b0a7f, rsp = 0xfffffe002366a7f0, rbp
> = 0xfffffe002366a7f0 ---
> strlen() at strlen+0x1f/frame 0xfffffe002366a7f0
> sbuf_cat() at sbuf_cat+0x15/frame 0xfffffe002366a810
> sysctl_devices() at sysctl_devices+0x104/frame 0xfffffe002366a8a0
> sysctl_root_handler_locked() at sysctl_root_handler_locked+0x91/frame
> 0xfffffe002366a8f0
> sysctl_root() at sysctl_root+0x249/frame 0xfffffe002366a970
> userland_sysctl() at userland_sysctl+0x170/frame 0xfffffe002366aa20
> sys___sysctl() at sys___sysctl+0x5f/frame 0xfffffe002366aad0
> amd64_syscall() at amd64_syscall+0x10c/frame 0xfffffe002366abf0
> fast_syscall_common() at fast_syscall_common+0xf8/frame 0xfffffe002366abf0
> --- syscall (202, FreeBSD ELF64, sys___sysctl), rip = 0x80041c0ea, rsp
> = 0x7fffffffda78, rbp = 0x7fffffffdab0 ---
> KDB: enter: panic
> [ thread pid 89 tid 100067 ]
> Stopped at      kdb_enter+0x37: movq    $0,0x7e2616(%rip)
>
>
> On 8/29/20, Warner Losh <imp@freebsd.org> wrote:
> > Author: imp
> > Date: Sat Aug 29 04:30:12 2020
> > New Revision: 364946
> > URL: https://svnweb.freebsd.org/changeset/base/364946
> >
> > Log:
> >   Move to using sbuf for some sysctl in newbus
> >
> >   Convert two different sysctl to using sbuf. First, for all the default
> >   sysctls we implement for each device driver that's attached. This is a
> >   pure sbuf conversion.
> >
> >   Second, convert sysctl_devices to fill its buffer with sbuf rather
> >   than a hand-rolled crappy thing I wrote years ago.
> >
> >   Reviewed by: cem, markj
> >   Differential Revision: https://reviews.freebsd.org/D26206
> >
> > Modified:
> >   head/sys/kern/subr_bus.c
> >
> > Modified: head/sys/kern/subr_bus.c
> >
> ==============================================================================
> > --- head/sys/kern/subr_bus.c  Sat Aug 29 04:30:06 2020        (r364945)
> > +++ head/sys/kern/subr_bus.c  Sat Aug 29 04:30:12 2020        (r364946)
> > @@ -260,36 +260,33 @@ enum {
> >  static int
> >  device_sysctl_handler(SYSCTL_HANDLER_ARGS)
> >  {
> > +     struct sbuf sb;
> >       device_t dev = (device_t)arg1;
> > -     const char *value;
> > -     char *buf;
> >       int error;
> >
> > -     buf = NULL;
> > +     sbuf_new_for_sysctl(&sb, NULL, 1024, req);
> >       switch (arg2) {
> >       case DEVICE_SYSCTL_DESC:
> > -             value = dev->desc ? dev->desc : "";
> > +             sbuf_cpy(&sb, dev->desc ? dev->desc : "");
> >               break;
> >       case DEVICE_SYSCTL_DRIVER:
> > -             value = dev->driver ? dev->driver->name : "";
> > +             sbuf_cpy(&sb, dev->driver ? dev->driver->name : "");
> >               break;
> >       case DEVICE_SYSCTL_LOCATION:
> > -             value = buf = malloc(1024, M_BUS, M_WAITOK | M_ZERO);
> > -             bus_child_location_str(dev, buf, 1024);
> > +             bus_child_location_sb(dev, &sb);
> >               break;
> >       case DEVICE_SYSCTL_PNPINFO:
> > -             value = buf = malloc(1024, M_BUS, M_WAITOK | M_ZERO);
> > -             bus_child_pnpinfo_str(dev, buf, 1024);
> > +             bus_child_pnpinfo_sb(dev, &sb);
> >               break;
> >       case DEVICE_SYSCTL_PARENT:
> > -             value = dev->parent ? dev->parent->nameunit : "";
> > +             sbuf_cpy(&sb, dev->parent ? dev->parent->nameunit : "");
> >               break;
> >       default:
> > +             sbuf_delete(&sb);
> >               return (EINVAL);
> >       }
> > -     error = SYSCTL_OUT_STR(req, value);
> > -     if (buf != NULL)
> > -             free(buf, M_BUS);
> > +     error = sbuf_finish(&sb);
> > +     sbuf_delete(&sb);
> >       return (error);
> >  }
> >
> > @@ -5464,13 +5461,13 @@ SYSCTL_PROC(_hw_bus, OID_AUTO, info,
> CTLTYPE_STRUCT
> > |
> >  static int
> >  sysctl_devices(SYSCTL_HANDLER_ARGS)
> >  {
> > +     struct sbuf             sb;
> >       int                     *name = (int *)arg1;
> >       u_int                   namelen = arg2;
> >       int                     index;
> >       device_t                dev;
> >       struct u_device         *udev;
> >       int                     error;
> > -     char                    *walker, *ep;
> >
> >       if (namelen != 2)
> >               return (EINVAL);
> > @@ -5501,34 +5498,21 @@ sysctl_devices(SYSCTL_HANDLER_ARGS)
> >       udev->dv_devflags = dev->devflags;
> >       udev->dv_flags = dev->flags;
> >       udev->dv_state = dev->state;
> > -     walker = udev->dv_fields;
> > -     ep = walker + sizeof(udev->dv_fields);
> > -#define CP(src)                                              \
> > -     if ((src) == NULL)                              \
> > -             *walker++ = '\0';                       \
> > -     else {                                          \
> > -             strlcpy(walker, (src), ep - walker);    \
> > -             walker += strlen(walker) + 1;           \
> > -     }                                               \
> > -     if (walker >= ep)                               \
> > -             break;
> > -
> > -     do {
> > -             CP(dev->nameunit);
> > -             CP(dev->desc);
> > -             CP(dev->driver != NULL ? dev->driver->name : NULL);
> > -             bus_child_pnpinfo_str(dev, walker, ep - walker);
> > -             walker += strlen(walker) + 1;
> > -             if (walker >= ep)
> > -                     break;
> > -             bus_child_location_str(dev, walker, ep - walker);
> > -             walker += strlen(walker) + 1;
> > -             if (walker >= ep)
> > -                     break;
> > -             *walker++ = '\0';
> > -     } while (0);
> > -#undef CP
> > -     error = SYSCTL_OUT(req, udev, sizeof(*udev));
> > +     sbuf_new(&sb, udev->dv_fields, sizeof(udev->dv_fields),
> SBUF_FIXEDLEN);
> > +     sbuf_cat(&sb, dev->nameunit);
> > +     sbuf_putc(&sb, '\0');
> > +     sbuf_cat(&sb, dev->desc);
> > +     sbuf_putc(&sb, '\0');
> > +     sbuf_cat(&sb, dev->driver != NULL ? dev->driver->name : '\0');
> > +     sbuf_putc(&sb, '\0');
> > +     bus_child_pnpinfo_sb(dev, &sb);
> > +     sbuf_putc(&sb, '\0');
> > +     bus_child_location_sb(dev, &sb);
> > +     sbuf_putc(&sb, '\0');
> > +     error = sbuf_finish(&sb);
> > +     if (error == 0)
> > +             error = SYSCTL_OUT(req, udev, sizeof(*udev));
> > +     sbuf_delete(&sb);
> >       free(udev, M_BUS);
> >       return (error);
> >  }
> >
>
>
> --
> Mateusz Guzik <mjguzik gmail.com>
>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqXtKhKhh33ovFQ4_a3tiesRi8-6ZuMTp0yW%2BMzkxWLzA>