On Fri, Dec 4, 2015 at 3:41 AM, YASUOKA Masahiko <[email protected]> wrote:
> On Sat, 10 Oct 2015 21:32:29 -0700
> Philip Guenther <[email protected]> wrote:
>> So:
>> * kill the 'mode' argument to PRIVSEP_OPEN and priv_open()
>> * fail a PRIVSEP_OPEN call if it's passed any flags other than
>> O_ACCMODE or O_NONBLOCK
>> * paranoia: mask O_CREAT when calling open() with only two arguments
>> * instead of using ioctl(FIONBIO) after the fact, pass O_NONBLOCK to
>> priv_open()
>
> As for "open with O_NONBLOCK and ioctl(FIONBIO)", see the diff
> following for example,
>
>> Index: usr.sbin/npppd/npppd/npppd_iface.c
>> ===================================================================
>> RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd_iface.c,v
>> retrieving revision 1.11
>> diff -u -p -r1.11 npppd_iface.c
>> --- usr.sbin/npppd/npppd/npppd_iface.c 20 Jul 2015 18:58:30 -0000
>> 1.11
>> +++ usr.sbin/npppd/npppd/npppd_iface.c 11 Oct 2015 04:17:40 -0000
>> @@ -283,15 +283,8 @@ npppd_iface_start(npppd_iface *_this)
>>
>> /* open device file */
>> snprintf(buf, sizeof(buf), "/dev/%s", _this->ifname);
>> - if ((_this->devf = priv_open(buf, O_RDWR, 0600)) < 0) {
>> + if ((_this->devf = priv_open(buf, O_RDWR | O_NONBLOCK)) < 0) {
>> npppd_iface_log(_this, LOG_ERR, "open(%s) failed: %m", buf);
>> - goto fail;
>> - }
>> -
>> - x = 1;
>> - if (ioctl(_this->devf, FIONBIO, &x) != 0) {
>> - npppd_iface_log(_this, LOG_ERR,
>> - "ioctl(FIONBIO) failed in %s(): %m", __func__);
>> goto fail;
>> }
>
> At least on tun, bpf or pppx they behave differently. Since
> open(,O_NONBLOCK) doesn't call ioctl(FIONBIO) internally and those
> device have/use their own flag.
>
> How should we fix this? Revert ioctl(FIONBIO), change the open(2)
> behavior or the devices behavior?
We should fix open(2); please try the diff below. Are you sure pppx
is affected? pppxioctl()'s FIONBIO case appears to be a no-op. I
certainly agree that bpf and tun are affected.
Philip Guenther
--- kern/vfs_syscalls.c 20 Nov 2015 07:11:52 -0000 1.245
+++ kern/vfs_syscalls.c 4 Dec 2015 17:46:34 -0000
@@ -45,6 +45,7 @@
#include <sys/conf.h>
#include <sys/sysctl.h>
#include <sys/file.h>
+#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/lock.h>
#include <sys/vnode.h>
@@ -951,6 +952,8 @@ doopenat(struct proc *p, int fd, const c
}
}
VOP_UNLOCK(vp, 0, p);
+ if (flags & O_NONBLOCK)
+ (*fp->f_ops->fo_ioctl)(fp, FIONBIO, (caddr_t)&flags, p);
*retval = indx;
FILE_SET_MATURE(fp, p);
out:
