Date: Wed, 14 Apr 2010 13:45:23 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: John Baldwin <jhb@FreeBSD.org> Cc: Alfred Perlstein <alfred@FreeBSD.org>, d@delphij.net, freebsd-arch@FreeBSD.org Subject: Re: _IOWR when errno != 0 Message-ID: <20100414130627.V12547@delplex.bde.org> In-Reply-To: <201004130853.16994.jhb@freebsd.org> References: <4BC39E93.7060906@delphij.net> <20100412233330.GC19003@elvis.mu.org> <4BC3BA48.9010009@delphij.net> <201004130853.16994.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 13 Apr 2010, John Baldwin wrote:
> On Monday 12 April 2010 8:26:48 pm Xin LI wrote:
>> On 2010/04/12 16:33, Alfred Perlstein wrote:
>>> * Xin LI <delphij@delphij.net> [100412 15:28] wrote:
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA1
>>>>
>>>> Hi,
>>>>
>>>> Is there a sane way to copyout ioctl request when the returning errno !=
>>>> 0? Looking at the code, currently, in sys/kern/sys_generic.c, we have:
No. You could just do it, but this would be insane since it would
just waste time.
>>>>
>>>> ===========
>>>> error = kern_ioctl(td, uap->fd, com, data);
>>>>
>>>> if (error == 0 && (com & IOC_OUT))
>>>> error = copyout(data, uap->data, (u_int)size);
>>>> ===========
>>>>
>>>> Is there any objection if I change it to something like:
>>>>
>>>> ===========
>>>> saved_error = kern_ioctl(td, uap->fd, com, data);
>>>>
>>>> if (com & IOC_OUT)
>>>> error = copyout(data, uap->data, (u_int)size);
>>>> if (saved_error)
>>>> error = saved_error;
>>>> ===========
errno != 0 means that the ioctl failed, so the contents of the output
buffer (output from the kernel) is indeterminate, so only broken
applications would look at it (except merely insane ones could look
at it and not use the results).
> Actually, this pattern of embedding an error is quite common. The mfi(4) and
> mpt(4) pass-thru ioctls to send firmware commands embed the return status of
> any firmware command in the structure that is passed in and out for example.
>
>>> I'm not sure this would work, it might seriously break userland
>>> compat. Have you looked around/queiried what the expected outcome
>>> is from a bad ioctl? By default the buffer will be zero'd this
>>> might be unexpected by apps. (all or nothing)
Such applications are broken. The error might occur at any point
in the syscall and apps have no way of telling where. Errors during
the copyout would cause a partial copy (!(all or nothing) unless partial
is actually nothing). With a partial copy, the changed bytes could
be anywhere in the copy, depending on the implementation.
>> Yes that's exactly why I'm asking, my understanding is that for normal
>> usages would be something like:
>>
>> if (ioctl(fd, SIOCSOMETHING, &req) < 0) {
Testing syscalls that return 0 on error using " < 0" is a normal style bug.
>> // do something to handle the error
>> } else {
>> // use data fed back from req
>> }
>>
>> In this case, I think the result would not be affected. Is there many
>> (if any) programs that don't bother to check return value of ioctl()?
Only broken ones.
>> Speaking for the userland buffer, for _IOR ioctls, the side effect would
>> be that userland would see a zeroed out 'req' structure (kernel buffer
>> gets zeroed out before calling kern_ioctl), or "half-baked" one (the
>> kernel code may have only written partial data). For _IOWR ioctls, the
>> side effect would be that the userland may get half-baked data.
Hmm, the kernel probably depends on the pre-zeroing, so that half-baked data
is not necessarily an error.
> You miss one important variation where the error handling involves adjusting
> the request and retrying (or submitting the same request to a different ioctl
> to handle renumbering conflicts, etc.). Other APIs such as sysctl(2) and
> setsockopt(2) can leave partial data, but the callers of those APIs expect
> that (and in fact, those APIs return the actual length of data that is copied
> out). ioctl(2) has not had that behavior, however, and I would find it
> surprising.
Yes, it has no general way of reporting partial success, and doing this for
special ioctls would be complicated. At the very least you would need to
add special error codes to distinguish normal failure (output buffer
indeterminate) from partial success (some bytes in output buffer valid,
and encode further details of the partialness).
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100414130627.V12547>