From owner-freebsd-arch@FreeBSD.ORG Wed Apr 14 03:45:42 2010 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EA9B71065673; Wed, 14 Apr 2010 03:45:41 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail04.syd.optusnet.com.au (mail04.syd.optusnet.com.au [211.29.132.185]) by mx1.freebsd.org (Postfix) with ESMTP id 1D0758FC08; Wed, 14 Apr 2010 03:45:40 +0000 (UTC) Received: from c211-30-173-227.carlnfd1.nsw.optusnet.com.au (c211-30-173-227.carlnfd1.nsw.optusnet.com.au [211.30.173.227]) by mail04.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o3E3jNcM027998 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 14 Apr 2010 13:45:24 +1000 Date: Wed, 14 Apr 2010 13:45:23 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: John Baldwin In-Reply-To: <201004130853.16994.jhb@freebsd.org> Message-ID: <20100414130627.V12547@delplex.bde.org> References: <4BC39E93.7060906@delphij.net> <20100412233330.GC19003@elvis.mu.org> <4BC3BA48.9010009@delphij.net> <201004130853.16994.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Alfred Perlstein , d@delphij.net, freebsd-arch@FreeBSD.org Subject: Re: _IOWR when errno != 0 X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 14 Apr 2010 03:45:42 -0000 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 [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