On Fri, 4 Dec 2015 09:47:46 -0800
Philip Guenther <guent...@gmail.com> wrote:
> On Fri, Dec 4, 2015 at 3:41 AM, YASUOKA Masahiko <yasu...@openbsd.org> wrote:
>> On Sat, 10 Oct 2015 21:32:29 -0700
>> Philip Guenther <guent...@gmail.com> 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.
You are right, sorry for my miunderstanding. Actually pppx is not
affected. npppd with tun (+ pipex.enable=0) was affected.
The diff fixes the problem.
Thanks,
--yasuoka
