From owner-svn-src-head@freebsd.org Sat Aug 29 10:38:26 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 53EFD3D4968; Sat, 29 Aug 2020 10:38:26 +0000 (UTC) (envelope-from meloun.michal@gmail.com) Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BdtGn4dMRz4HYq; Sat, 29 Aug 2020 10:38:25 +0000 (UTC) (envelope-from meloun.michal@gmail.com) Received: by mail-wr1-x441.google.com with SMTP id b18so1434635wrs.7; Sat, 29 Aug 2020 03:38:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:reply-to:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VVFV1yBTnxUIwL5QN4/6lDv/ifz3VmDDy3L8220G9ac=; b=eRHxeMHH9lQgTgp+5MRXZLeqGZ5H80Un31KractYxenW9t3K1Rs/JeqbiXaKcfiMMY yTxbWTrhv0BXlU++mh9ANHiq5zpdy+xdZcu1ECIt+15FAhQHYLXYMDpKC3B1RyNW1DYv maWjILeOGc5k+AVEw5pomFEowcNWjQhW20MlvnoOD93qjKQK+Ysznyxz4hIKmRvydC6H dXBRQSlva4RIiJ7Suzkkq4JWe9le4HQqWQvDjKegbsX66H/HmlPrPPLPnk83YfadwHh+ c0YQBLVlH0HnDesJWtIZDIbBvoUpk9gWKXJZOASNd0p1o4es5hPJwySl46gBvD4PbA6i TbVA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:reply-to:subject:to:cc:references:from :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=VVFV1yBTnxUIwL5QN4/6lDv/ifz3VmDDy3L8220G9ac=; b=q8pcZs+f0glhGIz9+4kNo1YpG/+mfVpkEIxLW1x97WlBI7o7DBGCH1eCPupR+edatC 9W07cfDbsFSmG9Rhds99eaW1+qtjz7fK5eKIp7TTJIk4Mbx9IXbaieLlNnzqKcSZGaQA eaQ3HRXMoisA4Bm1FsyymEDM7yCY3LePN2O/ynUrt7n3NuO8g7Hk8wASDBcV3o1RJB53 +33dMjE8CowUSKGHkXB3kkhrYMU+Bh2PHOsb62RuRMFf+nGPi/EXh1l3tVEogMCTM6zJ cPrlIy4RKtPR9nZF9Pbt17bDBP9QG1dUmYqA4jj+iQwXCxlt3+3gH00FxyYe15IQYo5R kWZg== X-Gm-Message-State: AOAM532qvvoR2hQLmYHBhELgbJ52Tf13davs4R2nbBpdrGAfuY8gW+OB vFiWw28c8GWk6CD/ptUIkSw2oGpVbFk= X-Google-Smtp-Source: ABdhPJz58qD3cLhCnTq7Z5ZYQNWjLPLJs0BsXMZXULjFFrV0jKMIeUpNFH4ge8R9bgcWAskVS13TSg== X-Received: by 2002:adf:dd51:: with SMTP id u17mr2955745wrm.355.1598697503911; Sat, 29 Aug 2020 03:38:23 -0700 (PDT) Received: from [88.208.79.100] (halouny.humusoft.cz. [88.208.79.100]) by smtp.gmail.com with ESMTPSA id t4sm3041141wre.30.2020.08.29.03.38.23 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 29 Aug 2020 03:38:23 -0700 (PDT) Sender: Michal Meloun Reply-To: meloun.michal@gmail.com Subject: Re: svn commit: r364946 - head/sys/kern To: Warner Losh , Mateusz Guzik Cc: Warner Losh , src-committers , svn-src-all , svn-src-head References: <202008290430.07T4UCM4007928@repo.freebsd.org> From: Michal Meloun Message-ID: Date: Sat, 29 Aug 2020 12:38:24 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4BdtGn4dMRz4HYq X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=gmail.com header.s=20161025 header.b=eRHxeMHH; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (mx1.freebsd.org: domain of melounmichal@gmail.com designates 2a00:1450:4864:20::441 as permitted sender) smtp.mailfrom=melounmichal@gmail.com X-Spamd-Result: default: False [-3.12 / 15.00]; HAS_REPLYTO(0.00)[meloun.michal@gmail.com]; RCVD_VIA_SMTP_AUTH(0.00)[]; FREEMAIL_FROM(0.00)[gmail.com]; R_SPF_ALLOW(-0.20)[+ip6:2a00:1450:4000::/36]; RCPT_COUNT_FIVE(0.00)[6]; RCVD_COUNT_THREE(0.00)[3]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[gmail.com:+]; DMARC_POLICY_ALLOW(-0.50)[gmail.com,none]; NEURAL_HAM_SHORT(-0.19)[-0.193]; FREEMAIL_TO(0.00)[bsdimp.com,gmail.com]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US]; MID_RHS_MATCH_FROM(0.00)[]; TAGGED_FROM(0.00)[]; DWL_DNSWL_NONE(0.00)[gmail.com:dkim]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.97)[-0.972]; R_DKIM_ALLOW(-0.20)[gmail.com:s=20161025]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-0.95)[-0.955]; MIME_GOOD(-0.10)[text/plain]; FREEMAIL_REPLYTO(0.00)[gmail.com]; REPLYTO_DOM_EQ_FROM_DOM(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[2a00:1450:4864:20::441:from]; RCVD_TLS_ALL(0.00)[]; MAILMAN_DEST(0.00)[svn-src-head,svn-src-all] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 29 Aug 2020 10:38:26 -0000 On 29.08.2020 12:04, Warner Losh wrote: > On Sat, Aug 29, 2020 at 1:09 AM Mateusz Guzik 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. It crashed on ofw based systems where some enumerated devices have not a suitable driver, see: --------------------------------------- sysctl_devices: nameunit: root0, descs: System root bus, driver: root sysctl_devices: nameunit: nexus0, descs: (null), driver: nexus sysctl_devices: nameunit: ofwbus0, descs: Open Firmware Device Tree, driver: ofwbus sysctl_devices: nameunit: pcib0, descs: Nvidia Integrated PCI/PCI-E Controller, driver: pcib sysctl_devices: nameunit: simplebus0, descs: Flattened device tree simple bus, driver: simplebus sysctl_devices: nameunit: gic0, descs: ARM Generic Interrupt Controller, driver: gic sysctl_devices: nameunit: (null), descs: (null), driver: sysctl_devices: nameunit: lic0, descs: (null), driver: lic sysctl_devices: nameunit: (null), descs: (null), driver: sysctl_devices: nameunit: car0, descs: Tegra Clock Driver, driver: car .... ---------------------------------------------------------------------- > Fixed in r364949.Confirmed. 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... > But there is another issue in device_sysctl_handler() (not analyzed yet): root@tegra210:~ # sysctl dev.cpu. dev.cpu.3.temperature: 50.5C dev.cpu.3panic: sbuf_clear makes no sense on sbuf 0xffff00006f21a528 with drain cpuid = 2 time = 1598696937 KDB: stack backtrace: db_trace_self() at db_fetch_ksymtab+0x164 pc = 0xffff0000006787f4 lr = 0xffff000000153400 sp = 0xffff00006f21a1b0 fp = 0xffff00006f21a3b0 db_fetch_ksymtab() at vpanic+0x198 pc = 0xffff000000153400 lr = 0xffff00000036b274 sp = 0xffff00006f21a3c0 fp = 0xffff00006f21a420 vpanic() at panic+0x44 pc = 0xffff00000036b274 lr = 0xffff00000036b018 sp = 0xffff00006f21a430 fp = 0xffff00006f21a4e0 panic() at sbuf_clear+0xa0 pc = 0xffff00000036b018 lr = 0xffff0000003c17c8 sp = 0xffff00006f21a4f0 fp = 0xffff00006f21a4f0 sbuf_clear() at sbuf_cpy+0x58 pc = 0xffff0000003c17c8 lr = 0xffff0000003c1ff0 sp = 0xffff00006f21a500 fp = 0xffff00006f21a500 sbuf_cpy() at _gone_in_dev+0x560 pc = 0xffff0000003c1ff0 lr = 0xffff0000003a9078 sp = 0xffff00006f21a510 fp = 0xffff00006f21a570 _gone_in_dev() at sbuf_new_for_sysctl+0x170 pc = 0xffff0000003a9078 lr = 0xffff00000037c1a8 sp = 0xffff00006f21a580 fp = 0xffff00006f21a5a0 sbuf_new_for_sysctl() at kernel_sysctl+0x36c pc = 0xffff00000037c1a8 lr = 0xffff00000037b63c sp = 0xffff00006f21a5b0 fp = 0xffff00006f21a630 kernel_sysctl() at userland_sysctl+0xf4 pc = 0xffff00000037b63c lr = 0xffff00000037bc5c sp = 0xffff00006f21a640 fp = 0xffff00006f21a6d0 userland_sysctl() at sys___sysctl+0x68 pc = 0xffff00000037bc5c lr = 0xffff00000037bb28 sp = 0xffff00006f21a6e0 fp = 0xffff00006f21a790 sys___sysctl() at do_el0_sync+0x4e0 pc = 0xffff00000037bb28 lr = 0xffff000000697918 sp = 0xffff00006f21a7a0 fp = 0xffff00006f21a830 do_el0_sync() at handle_el0_sync+0x90 pc = 0xffff000000697918 lr = 0xffff00000067aa24 sp = 0xffff00006f21a840 fp = 0xffff00006f21a980 handle_el0_sync() at 0x4047764c pc = 0xffff00000067aa24 lr = 0x000000004047764c sp = 0xffff00006f21a990 fp = 0x0000ffffffffc250 KDB: enter: panic [ thread pid 1263 tid 100092 ] Stopped at 0x40477fb4: undefined 54000042 > 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 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 >> >