On Fri, Sep 22, 2017 at 7:06 AM, Willem de Bruijn
<willemdebruijn.ker...@gmail.com> wrote:
>> @@ -2061,6 +2174,9 @@ static int tun_set_iff(struct net *net, struct file
>> *file, struct ifreq *ifr)
>> if (tfile->detached)
>> return -EINVAL;
>>
>> + if ((ifr->ifr_flags & IFF_NAPI_FRAGS) && !capable(CAP_NET_ADMIN))
>> + return -EPERM;
>> +
>
> This should perhaps be moved into the !dev branch, directly below the
> ns_capable check.
>
Hmm, does that mean fail only on creation but allow to attach if
exists? That would be wrong, isn't it? Correct me if I'm wrong but we
want to prevent both these scenarios if user does not have sufficient
privileges (i.e. NET_ADMIN in init-ns).
>> dev = __dev_get_by_name(net, ifr->ifr_name);
>> if (dev) {
>> if (ifr->ifr_flags & IFF_TUN_EXCL)
>> @@ -2185,6 +2301,9 @@ static int tun_set_iff(struct net *net, struct file
>> *file, struct ifreq *ifr)
>> tun->flags = (tun->flags & ~TUN_FEATURES) |
>> (ifr->ifr_flags & TUN_FEATURES);
>>
>> + if (!(tun->flags & IFF_NAPI) || (tun->flags & TUN_TYPE_MASK) !=
>> IFF_TAP)
>> + tun->flags = tun->flags & ~IFF_NAPI_FRAGS;
>> +
>
> Similarly, this check only need to be performed in that branch.
> Instead of reverting to non-frags mode, a tun_set_iff with the wrong
> set of flags should probably fail hard.
Yes, agree, wrong set of flags should fail hard and probably be done
before attach or open, no?
