Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 13 Apr 2010 08:53:16 -0400
From:      John Baldwin <jhb@freebsd.org>
To:        freebsd-arch@freebsd.org, d@delphij.net
Cc:        Alfred Perlstein <alfred@freebsd.org>
Subject:   Re: _IOWR when errno != 0
Message-ID:  <201004130853.16994.jhb@freebsd.org>
In-Reply-To: <4BC3BA48.9010009@delphij.net>
References:  <4BC39E93.7060906@delphij.net> <20100412233330.GC19003@elvis.mu.org> <4BC3BA48.9010009@delphij.net>

next in thread | previous in thread | raw e-mail | index | archive | help
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:
> >>
> >> ===========
> >>         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;
> >> ===========
> >
> > Is this for linux compat?
> 
> Do they do this way?  I'm not quite sure :-/
> 
> I got a bug report and am thinking about how to fix it, it seems that we
> do not have a generic way of returning an error number while giving some
> "hints" about the error at the same time, for the ioctl() call.  Adding
> an extra pointer to the request structure seems to be a last-resort way
> and sounds to be ugly.

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)
> 
> 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) {
> 		// 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()?
> 
> 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.

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.

-- 
John Baldwin



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201004130853.16994.jhb>