From owner-freebsd-new-bus@FreeBSD.ORG Fri Nov 6 16:21:11 2009 Return-Path: Delivered-To: freebsd-new-bus@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id AAD4E1065695; Fri, 6 Nov 2009 16:21:11 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 689588FC20; Fri, 6 Nov 2009 16:21:11 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id nA6GFenb085596; Fri, 6 Nov 2009 09:15:40 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Fri, 06 Nov 2009 09:15:43 -0700 (MST) Message-Id: <20091106.091543.2076840904.imp@bsdimp.com> To: attilio@FreeBSD.org From: "M. Warner Losh" In-Reply-To: <3bbf2fe10911060720m6d6919ffw91dcc5b6c1c2016a@mail.gmail.com> References: <3bbf2fe10911060720m6d6919ffw91dcc5b6c1c2016a@mail.gmail.com> X-Mailer: Mew version 5.2 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-new-bus@FreeBSD.org, scottl@FreeBSD.org, emaste@sandvine.com, jhb@FreeBSD.org Subject: Re: [PATCH] Buffer overflow in devclass_add_device() X-BeenThere: freebsd-new-bus@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: FreeBSD's new-bus architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Nov 2009 16:21:11 -0000 In message: <3bbf2fe10911060720m6d6919ffw91dcc5b6c1c2016a@mail.gmail.com> Attilio Rao writes: : A buffer overflow is possible in devclass_add_device(). : More specifically, the dev nameunit construction is based on the : assumption that the unit linked with the device is invariant but that : can change when calling devclass_alloc_unit() (because -1 is passed : or, more simply, because the unit choosen is beyond the table limits). : This results in a buffer overflow if the bug is too short on the : second snprintf(). : This patch should fix it: : http://www.freebsd.org/~attilio/Sandvine/STABLE_8/subr_bus/subr_bus.diff : : aiming for the max possible number of digits necessary. : This bug has been found by Sandvine Incorporated. : Please reivew. I don't see a problem with it, except you'd want -INT_MAX to be paranoid, since it is one character longer (or just add 1) :) However, it might be better to just allocate strlen(dc->name) + log10(INT_MAX) + 2 and not have snprintf do that calculation. But it doesn't look like there's a compile-time constant for that... Warner