From owner-freebsd-hackers@freebsd.org Fri Aug 30 04:03:32 2019 Return-Path: Delivered-To: freebsd-hackers@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 40C4DC0A04; Fri, 30 Aug 2019 04:03:32 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail105.syd.optusnet.com.au (mail105.syd.optusnet.com.au [211.29.132.249]) by mx1.freebsd.org (Postfix) with ESMTP id 46KQnZ3Cmyz4c3q; Fri, 30 Aug 2019 04:03:29 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail105.syd.optusnet.com.au (Postfix) with ESMTPS id 63005361D09; Fri, 30 Aug 2019 14:03:27 +1000 (AEST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eugene Grosbein cc: freebsd-security@freebsd.org, Freebsd hackers list Subject: Re: FreeBSD Security Advisory FreeBSD-SA-19:23.midi In-Reply-To: Message-ID: <20190830133855.W1405@besplex.bde.org> References: <20190820201257.7A9D41F8B7@freefall.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=P6RKvmIu c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=jpOVt7BSZ2e4Z31A5e1TngXxSK0=:19 a=kj9zAlcOel0A:10 a=i-w8XHbryAZmjjn50WgA:9 a=CjuIK1q_8ugA:10 X-Rspamd-Queue-Id: 46KQnZ3Cmyz4c3q X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of brde@optusnet.com.au designates 211.29.132.249 as permitted sender) smtp.mailfrom=brde@optusnet.com.au X-Spamd-Result: default: False [-3.26 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; RCVD_IN_DNSWL_LOW(-0.10)[249.132.29.211.list.dnswl.org : 127.0.5.1]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; R_SPF_ALLOW(-0.20)[+ip4:211.29.132.0/23:c]; FREEMAIL_FROM(0.00)[optusnet.com.au]; MIME_GOOD(-0.10)[text/plain]; DMARC_NA(0.00)[optusnet.com.au]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; TO_DN_SOME(0.00)[]; IP_SCORE_FREEMAIL(0.00)[]; TO_MATCH_ENVRCPT_SOME(0.00)[]; NEURAL_HAM_SHORT(-0.96)[-0.965,0]; IP_SCORE(0.00)[ip: (-5.25), ipnet: 211.28.0.0/14(-3.26), asn: 4804(-2.38), country: AU(0.01)]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[optusnet.com.au]; ASN(0.00)[asn:4804, ipnet:211.28.0.0/14, country:AU]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2] X-Mailman-Approved-At: Sat, 12 Oct 2019 23:39:36 +0000 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Fri, 30 Aug 2019 04:03:32 -0000 X-Original-Date: Fri, 30 Aug 2019 14:03:25 +1000 (EST) X-List-Received-Date: Fri, 30 Aug 2019 04:03:32 -0000 On Wed, 21 Aug 2019, Eugene Grosbein wrote: > 21.08.2019 3:12, FreeBSD Security Advisories wrote: > > [skip] > >> IV. Workaround >> >> No workaround is available. Custom kernels without "device sound" >> are not vulnerable. > > Is it true that there is no way to disable vulnerable and unneeded device driver > built in GENERIC other that through rebuilding the kernel? > > I remember that pre-4.x versions of FreeBSD had visual VGA-based pre-boot configurator Visual userconfig and command-line userconfug were in all versions of 4.x too. > allowing to disable any compiled-in device driver. Don't device.hints(5) or loader(8) have means to do so? Configuration was unimproved by hints, env and new-bus after 4.x. In 4.x and earlier, the irq and other parameters, and disable and other flags, were part of a formal syntax implemented at config(8) time using yacc and at kernel userconfig time more hackishly and at kernel visual userconfig time more guishlly. Now hints and env give a random mostly undocumented syntax. Even disable flags don't work right. New-bus allows more complicated or just larger topologies which are harder to control using disable flags. > These days GENERIC have LOTS of drivers and it's convenient but unsafe. It is hard to even find the list of (unattached) drivers, or get useful (fauling) probe messages for drivers that aren't used. I use the following patch mainly to fix sio and uart probing in uncontrollable or hard-coded order and/or precendence when both are statically configured. One must be disabled on a per-device basis, but even disabling doesn't work without this patch. The patch preserves some historical mistakes and adds some excessive verboseness about probe failures. I'm still waiting for jhb to reply to mails on 30 Oct 2015 and 23 Jan 2018 asking for a review of this patch or better a complete fix. XX Index: subr_bus.c XX =================================================================== XX --- subr_bus.c (revision 332488) XX +++ subr_bus.c (working copy) XX @@ -2079,6 +2079,12 @@ XX return (TAILQ_NEXT(last, link)); XX } XX XX +/* XX + * Keep probing disabled devices for now, in case this has beneficial side XX + * effects. XX + */ XX +static volatile int probe_rdisabled = 0; XX + XX /** XX * @internal XX */ XX @@ -2088,7 +2094,7 @@ XX devclass_t dc; XX driverlink_t best = NULL; XX driverlink_t dl; XX - int result, pri = 0; XX + int rdisabled, result, unit, pri = 0; XX int hasclass = (child->devclass != NULL); XX XX GIANT_REQUIRED; XX @@ -2139,8 +2145,27 @@ XX resource_int_value(dl->driver->name, child->unit, XX "flags", &child->devflags); XX XX - result = DEVICE_PROBE(child); XX + /* Record other state while the unit is valid. */ XX + unit = child->unit; XX + rdisabled = resource_disabled(dl->driver->name, unit); XX XX + /* See below for more details. */ XX + if (rdisabled) { XX + device_print_prettyname(dev); XX + if (probe_rdisabled) XX + device_printf(child, XX + "probing disabled device\n"); XX + else { XX + device_printf(child, XX + "disabled in probe by hints\n"); XX + device_disable(child); XX + } XX + } XX + if (rdisabled && !probe_rdisabled) XX + result = ENXIO; XX + else XX + result = DEVICE_PROBE(child); XX + XX /* Reset flags and devclass before the next probe. */ XX child->devflags = 0; XX if (!hasclass) XX @@ -2182,6 +2207,30 @@ XX } XX XX /* XX + * Ignore the result of probing a disabled device, XX + * so that disabled devices with higher priorities XX + * are not preferred, only to do nothing at attach XX + * time but complete their disablement and fail. XX + * This is not quite right since it loses the XX + * accidental (?) feature of being able to disable XX + * attaching a resource for all drivers by XX + * disabling it for one driver if there happens to XX + * one with highest priority (or equal highest, XX + * with the disabled one preferred because it is XX + * probed first. XX + */ XX + if (rdisabled) { XX + device_print_prettyname(dev); XX + /* XXX device_printf() fails -- child inval. */ XX + printf("%s%d: disabled in probe by hints\n", XX + dl->driver->name, unit); XX +#ifdef notyet /* XXX don't risk this with invalid child->unit */ XX + device_disable(child); XX +#endif XX + continue; XX + } XX + XX + /* XX * A priority lower than SUCCESS, remember the XX * best matching driver. Initialise the value XX * of pri for the first match. The rest is unrelated (keep the message out of the attach timing). But it seems reasonable to get entropy from probe time as well as attach timing, and that would include timing for spammish probe messages. XX @@ -2909,8 +2958,6 @@ XX } XX XX device_sysctl_init(dev); XX - if (!device_is_quiet(dev)) XX - device_print_child(dev->parent, dev); XX attachtime = get_cyclecount(); XX dev->state = DS_ATTACHING; XX if ((error = DEVICE_ATTACH(dev)) != 0) { XX @@ -2925,6 +2972,8 @@ XX return (error); XX } XX attachtime = get_cyclecount() - attachtime; XX + if (!device_is_quiet(dev)) XX + device_print_child(dev->parent, dev); XX /* XX * 4 bits per device is a reasonable value for desktop and server XX * hardware with good get_cyclecount() implementations, but WILL Bruce