On 7/28/15 9:25 AM, Andy Lutomirski wrote:
On Jul 27, 2015 11:33 AM, "David Ahern" <d...@cumulusnetworks.com> wrote:

Allow tasks to have a default device index for binding sockets. If set
the value is passed to all AF_INET/AF_INET6 sockets when they are created.


This is not intended to be a review of the concept.  I haven't thought
about whether the concept is a good idea, broken by design, or
whatever.  FWIW, if this were added to the kernel and didn't require
excessive privilege, I'd probably use it.  (I still don't really
understand why binding to a device requires privilege in the first
place, but, again, I haven't thought about it very much.)

The intent here is to restrict a task to only sending and receiving packets from a single network device. The device can be single ethernet interface, a stacked device (e.g, bond) or in our case a VRF device which restricts a task to interfaces (and hence network paths) associated with the VRF.


+#ifdef CONFIG_NET
+       case PR_SET_SK_BIND_DEV_IF:
+       {
+               struct net_device *dev;
+               int idx = (int) arg2;
+
+               if (!capable(CAP_NET_ADMIN))
+                       return -EPERM;
+

Can you either use ns_capable or add a comment as to why not?

will do.


Also, please return -EINVAL if unused args are nonzero.

ok.


+               if (idx) {
+                       dev = dev_get_by_index(me->nsproxy->net_ns, idx);
+                       if (!dev)
+                               return -EINVAL;
+                       dev_put(dev);
+               }
+               me->sk_bind_dev_if = idx;
+               break;
+       }
+       case PR_GET_SK_BIND_DEV_IF:
+       {
+               struct task_struct *tsk;
+               int sk_bind_dev_if = -EINVAL;
+
+               rcu_read_lock();
+               tsk = find_task_by_vpid(arg2);
+               if (tsk)
+                       sk_bind_dev_if = tsk->sk_bind_dev_if;

Why do you support different tasks here?  Could this use proc instead?

In this case we want to allow a separate process to determine if a task is restricted to a device.


The same -EINVAL issue applies.

Also, I think you need to hook setns and unshare to do something
reasonable when the task is bound to a device.

ack on both.

Thanks for the review,
David
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to