On Mon, Jan 18, 2016 at 06:56:29PM +0100, Sebastien Marie wrote:
> On Mon, Jan 18, 2016 at 06:05:31PM +0100, Alexandre Ratchov wrote:
> > On Mon, Jan 18, 2016 at 01:35:46PM +0100, Sebastien Marie wrote:
> > > On Mon, Jan 18, 2016 at 12:55:30PM +0100, Alexandre Ratchov wrote:
> > >
> > > I am unsure about returning 0 for something we know is wrong to do.
> >
> > heh, it's not wrong; disconnecting audio devices is a legitimate
> > use of them ;)
>
> I didn't want to said that the operation in a whole is wrong, but that
> making pledge_ioctl() returning 0 would be wrong as we know the
> operation shouldn't be allowed in normal case.
the operation _is_ allowed in the normal case; except that the
device is gone and the operation returns an error code indicating
that it failed.
> > > Isn't possible to return an error ? As example, calling
> > > ioctl(TIOCGWINSZ) on no-tty device return ENOTTY.
> > >
> >
> > Agreed, and it's same for chardevs: the ioctl syscall returns
> > uncondionnaly ENOTTY on disconnected devices: sys_ioctl() calls
> > vn_ioctl() which returns ENOTTY if vp->v_type == VBAD.
>
> hum ? I dunno if I was clear. Returning ENOTTY was an example. Does
> disconnecting audio devices makes ioctl(2) return ENOTTY ? I doubt.
ioctl() on any disconnected device returns ENOTTY, this is not
specific to audio.
>
> Modulo the ENOTTY error code (see previous comment), yes the purpose is
> to early return from pledge_ioctl(). pledge(2) permits to expose only a
> portion of deeper kernel code for a set of defined operations.
>
OK, I dislike the ENOTTY error code as well. The most appropriate
is EIO, as this is what read(2) and write(2) return in this case.
(but in this case pledged code returns EIO, while non-pledged code
returns ENOTTY. Not a big deal as the caller generally takes the
same code-path code paths in both cases).
does this looks better?
Index: kern_pledge.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.146
diff -u -p -u -p -r1.146 kern_pledge.c
--- kern_pledge.c 9 Jan 2016 06:13:43 -0000 1.146
+++ kern_pledge.c 18 Jan 2016 18:23:09 -0000
@@ -1205,10 +1205,13 @@ pledge_ioctl(struct proc *p, long com, s
case AUDIO_GETENC:
case AUDIO_SETFD:
case AUDIO_GETPROPS:
- if (fp->f_type == DTYPE_VNODE &&
- vp->v_type == VCHR &&
+ if (fp->f_type != DTYPE_VNODE)
+ break;
+ if (vp->v_type == VCHR &&
cdevsw[major(vp->v_rdev)].d_open == audioopen)
return (0);
+ if (vp->v_type == VBAD)
+ return (EIO);
}
#endif /* NAUDIO > 0 */
}
