Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

authored by

Thomas Graf and committed by
David S. Miller
cf7afbfe 89bf67f1

+53 -20
+8 -4
include/net/rtnetlink.h
··· 92 92 * specific netlink attributes. 93 93 * @get_link_af_size: Function to calculate size of address family specific 94 94 * netlink attributes exlusive the container attribute. 95 - * @parse_link_af: Function to parse a IFLA_AF_SPEC attribute and modify 96 - * net_device accordingly. 95 + * @validate_link_af: Validate a IFLA_AF_SPEC attribute, must check attr 96 + * for invalid configuration settings. 97 + * @set_link_af: Function to parse a IFLA_AF_SPEC attribute and modify 98 + * net_device accordingly. 97 99 */ 98 100 struct rtnl_af_ops { 99 101 struct list_head list; ··· 105 103 const struct net_device *dev); 106 104 size_t (*get_link_af_size)(const struct net_device *dev); 107 105 108 - int (*parse_link_af)(struct net_device *dev, 109 - const struct nlattr *attr); 106 + int (*validate_link_af)(const struct net_device *dev, 107 + const struct nlattr *attr); 108 + int (*set_link_af)(struct net_device *dev, 109 + const struct nlattr *attr); 110 110 }; 111 111 112 112 extern int __rtnl_af_register(struct rtnl_af_ops *ops);
+24 -5
net/core/rtnetlink.c
··· 1107 1107 return -EINVAL; 1108 1108 } 1109 1109 1110 + if (tb[IFLA_AF_SPEC]) { 1111 + struct nlattr *af; 1112 + int rem, err; 1113 + 1114 + nla_for_each_nested(af, tb[IFLA_AF_SPEC], rem) { 1115 + const struct rtnl_af_ops *af_ops; 1116 + 1117 + if (!(af_ops = rtnl_af_lookup(nla_type(af)))) 1118 + return -EAFNOSUPPORT; 1119 + 1120 + if (!af_ops->set_link_af) 1121 + return -EOPNOTSUPP; 1122 + 1123 + if (af_ops->validate_link_af) { 1124 + err = af_ops->validate_link_af(dev, 1125 + tb[IFLA_AF_SPEC]); 1126 + if (err < 0) 1127 + return err; 1128 + } 1129 + } 1130 + } 1131 + 1110 1132 return 0; 1111 1133 } 1112 1134 ··· 1378 1356 const struct rtnl_af_ops *af_ops; 1379 1357 1380 1358 if (!(af_ops = rtnl_af_lookup(nla_type(af)))) 1381 - continue; 1359 + BUG(); 1382 1360 1383 - if (!af_ops->parse_link_af) 1384 - continue; 1385 - 1386 - err = af_ops->parse_link_af(dev, af); 1361 + err = af_ops->set_link_af(dev, af); 1387 1362 if (err < 0) 1388 1363 goto errout; 1389 1364
+21 -5
net/ipv4/devinet.c
··· 1289 1289 [IFLA_INET_CONF] = { .type = NLA_NESTED }, 1290 1290 }; 1291 1291 1292 - static int inet_parse_link_af(struct net_device *dev, const struct nlattr *nla) 1292 + static int inet_validate_link_af(const struct net_device *dev, 1293 + const struct nlattr *nla) 1293 1294 { 1294 - struct in_device *in_dev = __in_dev_get_rcu(dev); 1295 1295 struct nlattr *a, *tb[IFLA_INET_MAX+1]; 1296 1296 int err, rem; 1297 1297 1298 - if (!in_dev) 1299 - return -EOPNOTSUPP; 1298 + if (dev && !__in_dev_get_rcu(dev)) 1299 + return -EAFNOSUPPORT; 1300 1300 1301 1301 err = nla_parse_nested(tb, IFLA_INET_MAX, nla, inet_af_policy); 1302 1302 if (err < 0) ··· 1313 1313 return -EINVAL; 1314 1314 } 1315 1315 } 1316 + 1317 + return 0; 1318 + } 1319 + 1320 + static int inet_set_link_af(struct net_device *dev, const struct nlattr *nla) 1321 + { 1322 + struct in_device *in_dev = __in_dev_get_rcu(dev); 1323 + struct nlattr *a, *tb[IFLA_INET_MAX+1]; 1324 + int rem; 1325 + 1326 + if (!in_dev) 1327 + return -EAFNOSUPPORT; 1328 + 1329 + if (nla_parse_nested(tb, IFLA_INET_MAX, nla, NULL) < 0) 1330 + BUG(); 1316 1331 1317 1332 if (tb[IFLA_INET_CONF]) { 1318 1333 nla_for_each_nested(a, tb[IFLA_INET_CONF], rem) ··· 1704 1689 .family = AF_INET, 1705 1690 .fill_link_af = inet_fill_link_af, 1706 1691 .get_link_af_size = inet_get_link_af_size, 1707 - .parse_link_af = inet_parse_link_af, 1692 + .validate_link_af = inet_validate_link_af, 1693 + .set_link_af = inet_set_link_af, 1708 1694 }; 1709 1695 1710 1696 void __init devinet_init(void)
-6
net/ipv6/addrconf.c
··· 3956 3956 return 0; 3957 3957 } 3958 3958 3959 - static int inet6_parse_link_af(struct net_device *dev, const struct nlattr *nla) 3960 - { 3961 - return -EOPNOTSUPP; 3962 - } 3963 - 3964 3959 static int inet6_fill_ifinfo(struct sk_buff *skb, struct inet6_dev *idev, 3965 3960 u32 pid, u32 seq, int event, unsigned int flags) 3966 3961 { ··· 4665 4670 .family = AF_INET6, 4666 4671 .fill_link_af = inet6_fill_link_af, 4667 4672 .get_link_af_size = inet6_get_link_af_size, 4668 - .parse_link_af = inet6_parse_link_af, 4669 4673 }; 4670 4674 4671 4675 /*