On 05/23/2013 07:45 PM, Michael S. Tsirkin wrote:
> On Thu, May 23, 2013 at 11:12:31AM +0800, Jason Wang wrote:
>> Though the queue were in fact created by open(), we still need to add this 
>> check
>> to be compatible with tuntap which can let mgmt software use a single API to
>> manage queues. This patch only validates the device name and moves the 
>> TUNSETIFF
>> to a helper.
>>
>> Signed-off-by: Jason Wang <jasow...@redhat.com>
>> ---
>>  drivers/net/macvtap.c |   50 
>> ++++++++++++++++++++++++++++++++++++++----------
>>  1 files changed, 39 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
>> index 5fd341c..e3f9344 100644
>> --- a/drivers/net/macvtap.c
>> +++ b/drivers/net/macvtap.c
>> @@ -867,6 +867,7 @@ out:
>>      return ret;
>>  }
>>  
>> +
> Please don't add empty lines like this :).

Sure.
>
>>  static struct macvlan_dev *macvtap_get_vlan(struct macvtap_queue *q)
>>  {
>>      struct macvlan_dev *vlan;
>> @@ -884,6 +885,43 @@ static void macvtap_put_vlan(struct macvlan_dev *vlan)
>>      dev_put(vlan->dev);
>>  }
>>  
>> +static int macvtap_set_iff(struct file *file, struct ifreq __user *ifr_u)
>> +{
>> +    struct macvtap_queue *q = file->private_data;
>> +    struct net *net = current->nsproxy->net_ns;
>> +    struct inode *inode = file_inode(file);
>> +    struct net_device *dev, *dev2;
>> +    struct ifreq ifr;
>> +
>> +    if (copy_from_user(&ifr, ifr_u, sizeof(struct ifreq)))
>> +            return -EFAULT;
>> +
>> +    if (ifr.ifr_flags & IFF_MULTI_QUEUE) {
> So why do we validate the name?
> Pls add a comment.

Ok. The validation is to prevent the userspace create the queues on the
wrong device. Not sure whether or not this is needed for single queue case.
>
>> +            dev = __dev_get_by_name(net, ifr.ifr_name);
>> +            if (!dev)
>> +                    return -EINVAL;
>> +
>> +            dev2 = dev_get_by_macvtap_minor(iminor(inode));
>> +            if (!dev2)
>> +                    return -EINVAL;
>> +
>> +            if (dev != dev2) {
>> +                    dev_put(dev2);
>> +                    return -EINVAL;
>> +            }
>> +
>> +            dev_put(dev2);
>> +    }
>> +
>> +    if ((ifr.ifr_flags & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
>> +        (IFF_NO_PI | IFF_TAP))
>> +            return -EINVAL;
>> +    else
>> +            q->flags = ifr.ifr_flags;
>> +
>> +    return 0;
>> +}
>> +
>>  /*
>>   * provide compatibility with generic tun/tap interface
>>   */
>> @@ -902,17 +940,7 @@ static long macvtap_ioctl(struct file *file, unsigned 
>> int cmd,
>>  
>>      switch (cmd) {
>>      case TUNSETIFF:
>> -            /* ignore the name, just look at flags */
>> -            if (get_user(u, &ifr->ifr_flags))
>> -                    return -EFAULT;
>> -
>> -            ret = 0;
>> -            if ((u & ~IFF_VNET_HDR) != (IFF_NO_PI | IFF_TAP))
>> -                    ret = -EINVAL;
>> -            else
>> -                    q->flags = u;
>> -
>> -            return ret;
>> +            return macvtap_set_iff(file, ifr);
>>  
>>      case TUNGETIFF:
>>              vlan = macvtap_get_vlan(q);
>> -- 
>> 1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to