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

[Bruce Evans]

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:

>> SYSCTL_I() works even better that I first thought.  It automatically
>> gives support for all typedefed integral types.  No SYSCTL_FOO_T()s
>> ...

Grrr, my sentence breaks of 2 spaces are being echoed as 1 space and 1
hard \xa0.

> 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.  The bug affects mainly
cases where there is a kernel type mismatch, like using sysctl_handle_int()
on a 64-bit variable.  I forget how short reads were not detected as
errors.  It still takes the size in the sysctl data to be for 64 bits
for this error to be detectable for either read or write.  But your
recent changes should fix all such type mismatches.

> 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.

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.

> $WORK has gotten busy so I haven't had a chance to work on this much
> but I'm hopeful that the missus will watch the kids this weekend and
> allow me to hack.

You seem to have found time :-).

Bruce