rtnl: make link af-specific updates atomic

As David pointed out correctly, updates to af-specific attributes
are currently not atomic. If multiple changes are requested and
one of them fails, previous updates may have been applied already
leaving the link behind in a undefined state.

This patch splits the function parse_link_af() into two functions
validate_link_af() and set_link_at(). validate_link_af() is placed
to validate_linkmsg() check for errors as early as possible before
any changes to the link have been made. set_link_af() is called to
commit the changes later.

This method is not fail proof, while it is currently sufficient
to make set_link_af() inerrable and thus 100% atomic, the
validation function method will not be able to detect all error
scenarios in the future, there will likely always be errors
depending on states which are f.e. not protected by rtnl_mutex
and thus may change between validation and setting.

Also, instead of silently ignoring unknown address families and
config blocks for address families which did not register a set
function the errors EAFNOSUPPORT respectively EOPNOSUPPORT are
returned to avoid comitting 4 out of 5 update requests without
notifying the user.

Signed-off-by: Thomas Graf <tgraf@infradead.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index 35be0bbcd7da03d4587098b871923d42e1618d6c..4093ca78cf60f997e5c6a3e4eb845766ec56d6ef 100644 (file)
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -92,8 +92,10 @@ extern void  rtnl_link_unregister(struct rtnl_link_ops *ops);
  *                    specific netlink attributes.
  *     @get_link_af_size: Function to calculate size of address family specific
  *                        netlink attributes exlusive the container attribute.
- *     @parse_link_af: Function to parse a IFLA_AF_SPEC attribute and modify
- *                     net_device accordingly.
+ *     @validate_link_af: Validate a IFLA_AF_SPEC attribute, must check attr
+ *                        for invalid configuration settings.
+ *     @set_link_af: Function to parse a IFLA_AF_SPEC attribute and modify
+ *                   net_device accordingly.
  */
 struct rtnl_af_ops {
        struct list_head        list;
@@ -103,8 +105,10 @@ struct rtnl_af_ops {
                                                const struct net_device *dev);
        size_t                  (*get_link_af_size)(const struct net_device *dev);
 
-       int                     (*parse_link_af)(struct net_device *dev,
-                                                const struct nlattr *attr);
+       int                     (*validate_link_af)(const struct net_device *dev,
+                                                   const struct nlattr *attr);
+       int                     (*set_link_af)(struct net_device *dev,
+                                              const struct nlattr *attr);
 };
 
 extern int     __rtnl_af_register(struct rtnl_af_ops *ops);

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index bf69e5871b1a3a891a200bebe25b85d10b6a02f2..750db57f3bb3e628b44a481c11eea954d21ce88d 100644 (file)
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1107,6 +1107,28 @@ static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[])
                        return -EINVAL;
        }
 
+       if (tb[IFLA_AF_SPEC]) {
+               struct nlattr *af;
+               int rem, err;
+
+               nla_for_each_nested(af, tb[IFLA_AF_SPEC], rem) {
+                       const struct rtnl_af_ops *af_ops;
+
+                       if (!(af_ops = rtnl_af_lookup(nla_type(af))))
+                               return -EAFNOSUPPORT;
+
+                       if (!af_ops->set_link_af)
+                               return -EOPNOTSUPP;
+
+                       if (af_ops->validate_link_af) {
+                               err = af_ops->validate_link_af(dev,
+                                                       tb[IFLA_AF_SPEC]);
+                               if (err < 0)
+                                       return err;
+                       }
+               }
+       }
+
        return 0;
 }
 
@@ -1356,12 +1378,9 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
                        const struct rtnl_af_ops *af_ops;
 
                        if (!(af_ops = rtnl_af_lookup(nla_type(af))))
-                               continue;
-
-                       if (!af_ops->parse_link_af)
-                               continue;
+                               BUG();
 
-                       err = af_ops->parse_link_af(dev, af);
+                       err = af_ops->set_link_af(dev, af);
                        if (err < 0)
                                goto errout;
 

diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 71afc26c2df87996ce605d721f84cf3da47aaed2..d9f71bae45c40ddf0231c12164dea85d32c2b883 100644 (file)
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1289,14 +1289,14 @@ static const struct nla_policy inet_af_policy[IFLA_INET_MAX+1] = {
        [IFLA_INET_CONF]        = { .type = NLA_NESTED },
 };
 
-static int inet_parse_link_af(struct net_device *dev, const struct nlattr *nla)
+static int inet_validate_link_af(const struct net_device *dev,
+                                const struct nlattr *nla)
 {
-       struct in_device *in_dev = __in_dev_get_rcu(dev);
        struct nlattr *a, *tb[IFLA_INET_MAX+1];
        int err, rem;
 
-       if (!in_dev)
-               return -EOPNOTSUPP;
+       if (dev && !__in_dev_get_rcu(dev))
+               return -EAFNOSUPPORT;
 
        err = nla_parse_nested(tb, IFLA_INET_MAX, nla, inet_af_policy);
        if (err < 0)
@@ -1314,6 +1314,21 @@ static int inet_parse_link_af(struct net_device *dev, const struct nlattr *nla)
                }
        }
 
+       return 0;
+}
+
+static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla)
+{
+       struct in_device *in_dev = __in_dev_get_rcu(dev);
+       struct nlattr *a, *tb[IFLA_INET_MAX+1];
+       int rem;
+
+       if (!in_dev)
+               return -EAFNOSUPPORT;
+
+       if (nla_parse_nested(tb, IFLA_INET_MAX, nla, NULL) < 0)
+               BUG();
+
        if (tb[IFLA_INET_CONF]) {
                nla_for_each_nested(a, tb[IFLA_INET_CONF], rem)
                        ipv4_devconf_set(in_dev, nla_type(a), nla_get_u32(a));
@@ -1689,7 +1704,8 @@ static struct rtnl_af_ops inet_af_ops = {
        .family           = AF_INET,
        .fill_link_af     = inet_fill_link_af,
        .get_link_af_size = inet_get_link_af_size,
-       .parse_link_af    = inet_parse_link_af,
+       .validate_link_af = inet_validate_link_af,
+       .set_link_af      = inet_set_link_af,
 };
 
 void __init devinet_init(void)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4cf760598c2aa674bb9589b6d6d99c11d1c459fb..1023ad0d2b15e70797ed343cdfae0b5db41886b8 100644 (file)
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3956,11 +3956,6 @@ static int inet6_fill_link_af(struct sk_buff *skb, const struct net_device *dev)
        return 0;
 }
 
-static int inet6_parse_link_af(struct net_device *dev, const struct nlattr *nla)
-{
-       return -EOPNOTSUPP;
-}
-
 static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev,
                             u32 pid, u32 seq, int event, unsigned int flags)
 {
@@ -4670,7 +4665,6 @@ static struct rtnl_af_ops inet6_ops = {
        .family           = AF_INET6,
        .fill_link_af     = inet6_fill_link_af,
        .get_link_af_size = inet6_get_link_af_size,
-       .parse_link_af    = inet6_parse_link_af,
 };
 
 /*