svn commit: r217369 - in head/sys: cam/scsi sys

Sun Jan 16 05:34:04 UTC 2011

On Sat, Jan 15, 2011 at 7:06 PM, Bruce Evans <brde at optusnet.com.au> wrote:
> On Sat, 15 Jan 2011 mdf at freebsd.org wrote:
>> On Sat, Jan 15, 2011 at 6:55 AM, Bruce Evans <brde at optusnet.com.au> wrote:
>
>> The printing is done entirely in user-space, so it's not too bad.  I
>> 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.  Anything smaller should be copyout(9)'d as a 4 bit
>> quantity.
>
> I think it shouldn't be converted in the kernel.  Applications using
> sysctl already have to deal with fields shorter than int in structs
> returned by sysctl().  They don't need to deal with integers shorter
> than int only since sysctl() doesn't support such integers yet.  Short
> 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.  The first is sysctl(8) which
>> asks for the size when reading and manages it.  The 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.  It 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?  I guess they are not-so-hard-coding data types as
> int and long, etc., and hard-coding them as int32_t and int64_t.  Then
> 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.  Suppose there is a size
> mismatch causing an application tries to write 32 bits to a 64-bit
> kernel variable.  Then no error is detected, and the kernel variable
> ois left with garbage in its top bits.  The error detection last worked
> with 4.4BSD sysctl, and is still documented to work:
>
> %      [EINVAL]           A non-null newp is given and its specified length
> in
> %                         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.  The application might be probing for a size
> that works.  Then it shouldn't care what size the value is returned in,
> provided the value fits.  Writing is only slightly more delicate.  The
> application must use a size in which the value fits.  Then the kernel
> can expand the size if necessary.  It just has to be careful to fill
> the top bits and not have sign extension bugs.  sysctl_handle_i() can
> probably handle both directions.  Lower-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).  An 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.  Integers
> 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.  Just try to convert
> to whatever size the application is trying to use.  Don't even force >= 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. :-)