From owner-svn-src-head@FreeBSD.ORG Sun Jan 16 05:34:03 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F0E82106566B; Sun, 16 Jan 2011 05:34:03 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-iy0-f182.google.com (mail-iy0-f182.google.com [209.85.210.182]) by mx1.freebsd.org (Postfix) with ESMTP id 848E38FC12; Sun, 16 Jan 2011 05:34:03 +0000 (UTC) Received: by iyb26 with SMTP id 26so3806490iyb.13 for ; Sat, 15 Jan 2011 21:34:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=ilktLi3nUP4i77h4pctCkZiZqxCA90zG8fOk1NHzf0k=; b=htYJRD0ytv/0xoh6X6kckjoRxaiqPwfartwIkv1K90AsRHPNzzFkgrm5iiFsPZV2+J Vn/EFKfx5PLyQ+o4RVs8YpFsamb6h3zeJzZqs6s3m7cS89SxcKudh9ZyURczRT1MdloA EwKcw6xYFeC5fItumz+YwJ3jZ8KjTlhhkEJsw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=UK6XEBF4bI/WQB+0t6Uhg+BCiIXCjLaJe/mw/GdUlv2TjrhnZOkyhXu0g9aRgwWUAh oTrFTripS60ACarxrQPCMhmTHeCY3qiFVBV9Jr9HvUDfD4xWpFtGI2iTy3pZiImHl2EK 5buDD15VddK1Il1xEoNWMNx9P9ERBDiAC8SdM= MIME-Version: 1.0 Received: by 10.231.171.197 with SMTP id i5mr2651662ibz.54.1295156042774; Sat, 15 Jan 2011 21:34:02 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.231.160.147 with HTTP; Sat, 15 Jan 2011 21:34:02 -0800 (PST) In-Reply-To: <20110116132842.W10642@besplex.bde.org> References: <201101131820.p0DIKXip059402@svn.freebsd.org> <20110114174719.D28159@besplex.bde.org> <20110115133929.D16210@besplex.bde.org> <20110115170529.T16715@besplex.bde.org> <20110116013102.E21684@besplex.bde.org> <20110116132842.W10642@besplex.bde.org> Date: Sat, 15 Jan 2011 21:34:02 -0800 X-Google-Sender-Auth: aT8_YsBo6Qa6RNyiJ9AjZiAvgAo Message-ID: From: mdf@FreeBSD.org To: Bruce Evans Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Garrett Cooper Subject: Re: svn commit: r217369 - in head/sys: cam/scsi sys X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 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: Sun, 16 Jan 2011 05:34:04 -0000 On Sat, Jan 15, 2011 at 7:06 PM, Bruce Evans wrote: > On Sat, 15 Jan 2011 mdf@freebsd.org wrote: >> On Sat, Jan 15, 2011 at 6:55 AM, Bruce Evans wrot= e: > >> The printing is done entirely in user-space, so it's not too bad. =A0I >> had figured to upcast everything to [u]intmax_t anyways, and what to >> cast from would just be [u]int32_t and [u]int64_t depending on >> reported size. =A0Anything smaller should be copyout(9)'d as a 4 bit >> quantity. > > I think it shouldn't be converted in the kernel. =A0Applications using > sysctl already have to deal with fields shorter than int in structs > returned by sysctl(). =A0They don't need to deal with integers shorter > than int only since sysctl() doesn't support such integers yet. =A0Short > integers could still be returned as essentially themselves by packing > them into a struct with nothing else. > >> But, there's two factors at play here. =A0The first is sysctl(8) which >> asks for the size when reading and manages it. =A0The second is >> sysctl(2) where we have code like SCTL_MASK32 so a long on a 64-bit >> kernel looks like a long to a 32-bit app. =A0It would be simpler to >> expose this as an 8-byte quantity on 64-bit kernel, and sysctl(8) will >> deal with this just fine, but that would break some uses of sysctl(2). > > What are the uses? =A0I guess they are not-so-hard-coding data types as > int and long, etc., and hard-coding them as int32_t and int64_t. =A0Then > you get an error when the kernel returns an unexpected length, but > unexpected lengths are expected for 32-bit apps on 64-bit kernels. > > BTW, short writes are still horribly broken. =A0Suppose there is a size > mismatch causing an application tries to write 32 bits to a 64-bit > kernel variable. =A0Then no error is detected, and the kernel variable > ois left with garbage in its top bits. =A0The error detection last worked > with 4.4BSD sysctl, and is still documented to work: > > % =A0 =A0 =A0[EINVAL] =A0 =A0 =A0 =A0 =A0 A non-null newp is given and it= s specified length > in > % =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 newlen is too large or = too small. > > I think the case where newlen is too large still works. > > I guess the bug is potentially larger for 32 bit compat applications, > but it is rarely seen for those too since only system management > applications should be writing to kernel variables and there is no > reason to run such applications in 32 bit mode. At Isilon all of userland is still 32-bit only. The reason is that for a while we supported 32-bit and 64-bit kernels in the field, and in fact for various reasons on install the 32-bit kernel always came up first and a reboot after install was forced for 64-bit capable hardware. But we needed the same userspace to work on both kernels. >> However, I *think* it is safe to check the sysctl_req's oldidx/oldlen >> to see what size the user-space expects and cast a kernel long to int >> before SYSCTL_OUT. > > Maybe if the value fits. =A0The application might be probing for a size > that works. =A0Then it shouldn't care what size the value is returned in, > provided the value fits. =A0Writing is only slightly more delicate. =A0Th= e > application must use a size in which the value fits. =A0Then the kernel > can expand the size if necessary. =A0It just has to be careful to fill > the top bits and not have sign extension bugs. =A0sysctl_handle_i() can > probably handle both directions. =A0Lower-level sysctl code should still > return an error if there is a size mismatch. Probing for a size that works should be passing in NULL for the old ptr to get a hint, and for scalars the sie doesn't change. It would be a somewhat malformed app that probes for the size anywhere below e.g. 512 bytes or 1 page. > If a value doesn't fit, then the application will just have to detect > the error and try a larger size (or fail if it doesn't support larger > size). =A0An example might be a 32-bit app reading 64-bit kernel pointers= . > These probably have the high bits set, so they won't fit in 32-bit sizes. > But the application can easily read them using 64-bit sizes. =A0Integers > and pointers larger than 64 bits would be more interesting, since a > compat application might not have any integer data type larger enough > to represent them. > > So I changed my mind about converting in the kernel. =A0Just try to conve= rt > to whatever size the application is trying to use. =A0Don't even force >= =3D 32 > bit sizes on applications. The proposed code does attempt to convert to whatever the app uses, but it assumes the app is using at least a 32-bit quantity. :-) More on the other thread. Thanks, matthew