Commit cc4bdef2 authored by Paolo Abeni's avatar Paolo Abeni

Merge branch 'rtnetlink-improve-alt_ifname-config-and-fix-dangerous-group-usage'

Florent Fourcot says:

====================
rtnetlink: improve ALT_IFNAME config and fix dangerous GROUP usage

First commit forbids dangerous calls when both IFNAME and GROUP are
given, since it can introduce unexpected behaviour when IFNAME does not
match any interface.

Second patch achieves primary goal of this patchset to fix/improve
IFLA_ALT_IFNAME attribute, since previous code was never working for
newlink/setlink. ip-link command is probably getting interface index
before, and was not using this feature.

Last two patches are improving error code on corner cases.

Changes in v2:
  * Remove ifname argument in rtnl_dev_get/do_setlink
    functions (simplify code)
  * Use a boolean to avoid condition duplication in __rtnl_newlink

Changes in v3:
  * Simplify rtnl_dev_get signature

Changes in v4:
  * Rename link_lookup to link_specified

Changes in v5:
  * Re-order patches
====================

Link: https://lore.kernel.org/r/20220415165330.10497-1-florent.fourcot@wifirst.frSigned-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parents 8b11c35d b6177d32
...@@ -2644,17 +2644,23 @@ static int do_set_proto_down(struct net_device *dev, ...@@ -2644,17 +2644,23 @@ static int do_set_proto_down(struct net_device *dev,
static int do_setlink(const struct sk_buff *skb, static int do_setlink(const struct sk_buff *skb,
struct net_device *dev, struct ifinfomsg *ifm, struct net_device *dev, struct ifinfomsg *ifm,
struct netlink_ext_ack *extack, struct netlink_ext_ack *extack,
struct nlattr **tb, char *ifname, int status) struct nlattr **tb, int status)
{ {
const struct net_device_ops *ops = dev->netdev_ops; const struct net_device_ops *ops = dev->netdev_ops;
char ifname[IFNAMSIZ];
int err; int err;
err = validate_linkmsg(dev, tb, extack); err = validate_linkmsg(dev, tb, extack);
if (err < 0) if (err < 0)
return err; return err;
if (tb[IFLA_IFNAME])
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
else
ifname[0] = '\0';
if (tb[IFLA_NET_NS_PID] || tb[IFLA_NET_NS_FD] || tb[IFLA_TARGET_NETNSID]) { if (tb[IFLA_NET_NS_PID] || tb[IFLA_NET_NS_FD] || tb[IFLA_TARGET_NETNSID]) {
const char *pat = ifname && ifname[0] ? ifname : NULL; const char *pat = ifname[0] ? ifname : NULL;
struct net *net; struct net *net;
int new_ifindex; int new_ifindex;
...@@ -3010,21 +3016,16 @@ static int do_setlink(const struct sk_buff *skb, ...@@ -3010,21 +3016,16 @@ static int do_setlink(const struct sk_buff *skb,
} }
static struct net_device *rtnl_dev_get(struct net *net, static struct net_device *rtnl_dev_get(struct net *net,
struct nlattr *ifname_attr, struct nlattr *tb[])
struct nlattr *altifname_attr, {
char *ifname) char ifname[ALTIFNAMSIZ];
{
char buffer[ALTIFNAMSIZ]; if (tb[IFLA_IFNAME])
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
if (!ifname) { else if (tb[IFLA_ALT_IFNAME])
ifname = buffer; nla_strscpy(ifname, tb[IFLA_ALT_IFNAME], ALTIFNAMSIZ);
if (ifname_attr) else
nla_strscpy(ifname, ifname_attr, IFNAMSIZ); return NULL;
else if (altifname_attr)
nla_strscpy(ifname, altifname_attr, ALTIFNAMSIZ);
else
return NULL;
}
return __dev_get_by_name(net, ifname); return __dev_get_by_name(net, ifname);
} }
...@@ -3037,7 +3038,6 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3037,7 +3038,6 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net_device *dev; struct net_device *dev;
int err; int err;
struct nlattr *tb[IFLA_MAX+1]; struct nlattr *tb[IFLA_MAX+1];
char ifname[IFNAMSIZ];
err = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFLA_MAX, err = nlmsg_parse_deprecated(nlh, sizeof(*ifm), tb, IFLA_MAX,
ifla_policy, extack); ifla_policy, extack);
...@@ -3048,17 +3048,12 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3048,17 +3048,12 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err < 0) if (err < 0)
goto errout; goto errout;
if (tb[IFLA_IFNAME])
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
else
ifname[0] = '\0';
err = -EINVAL; err = -EINVAL;
ifm = nlmsg_data(nlh); ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0) if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index); dev = __dev_get_by_index(net, ifm->ifi_index);
else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
dev = rtnl_dev_get(net, NULL, tb[IFLA_ALT_IFNAME], ifname); dev = rtnl_dev_get(net, tb);
else else
goto errout; goto errout;
...@@ -3067,7 +3062,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3067,7 +3062,7 @@ static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
goto errout; goto errout;
} }
err = do_setlink(skb, dev, ifm, extack, tb, ifname, 0); err = do_setlink(skb, dev, ifm, extack, tb, 0);
errout: errout:
return err; return err;
} }
...@@ -3156,15 +3151,14 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3156,15 +3151,14 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0) if (ifm->ifi_index > 0)
dev = __dev_get_by_index(tgt_net, ifm->ifi_index); dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
dev = rtnl_dev_get(net, tb[IFLA_IFNAME], dev = rtnl_dev_get(net, tb);
tb[IFLA_ALT_IFNAME], NULL);
else if (tb[IFLA_GROUP]) else if (tb[IFLA_GROUP])
err = rtnl_group_dellink(tgt_net, nla_get_u32(tb[IFLA_GROUP])); err = rtnl_group_dellink(tgt_net, nla_get_u32(tb[IFLA_GROUP]));
else else
goto out; goto out;
if (!dev) { if (!dev) {
if (tb[IFLA_IFNAME] || ifm->ifi_index > 0) if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME] || ifm->ifi_index > 0)
err = -ENODEV; err = -ENODEV;
goto out; goto out;
...@@ -3299,7 +3293,7 @@ static int rtnl_group_changelink(const struct sk_buff *skb, ...@@ -3299,7 +3293,7 @@ static int rtnl_group_changelink(const struct sk_buff *skb,
for_each_netdev_safe(net, dev, aux) { for_each_netdev_safe(net, dev, aux) {
if (dev->group == group) { if (dev->group == group) {
err = do_setlink(skb, dev, ifm, extack, tb, NULL, 0); err = do_setlink(skb, dev, ifm, extack, tb, 0);
if (err < 0) if (err < 0)
return err; return err;
} }
...@@ -3326,6 +3320,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3326,6 +3320,7 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
struct ifinfomsg *ifm; struct ifinfomsg *ifm;
char ifname[IFNAMSIZ]; char ifname[IFNAMSIZ];
struct nlattr **data; struct nlattr **data;
bool link_specified;
int err; int err;
#ifdef CONFIG_MODULES #ifdef CONFIG_MODULES
...@@ -3340,18 +3335,17 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3340,18 +3335,17 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err < 0) if (err < 0)
return err; return err;
if (tb[IFLA_IFNAME])
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
else
ifname[0] = '\0';
ifm = nlmsg_data(nlh); ifm = nlmsg_data(nlh);
if (ifm->ifi_index > 0) if (ifm->ifi_index > 0) {
link_specified = true;
dev = __dev_get_by_index(net, ifm->ifi_index); dev = __dev_get_by_index(net, ifm->ifi_index);
else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) } else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) {
dev = rtnl_dev_get(net, NULL, tb[IFLA_ALT_IFNAME], ifname); link_specified = true;
else dev = rtnl_dev_get(net, tb);
} else {
link_specified = false;
dev = NULL; dev = NULL;
}
master_dev = NULL; master_dev = NULL;
m_ops = NULL; m_ops = NULL;
...@@ -3450,15 +3444,20 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3450,15 +3444,20 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
status |= DO_SETLINK_NOTIFY; status |= DO_SETLINK_NOTIFY;
} }
return do_setlink(skb, dev, ifm, extack, tb, ifname, status); return do_setlink(skb, dev, ifm, extack, tb, status);
} }
if (!(nlh->nlmsg_flags & NLM_F_CREATE)) { if (!(nlh->nlmsg_flags & NLM_F_CREATE)) {
if (ifm->ifi_index == 0 && tb[IFLA_GROUP]) /* No dev found and NLM_F_CREATE not set. Requested dev does not exist,
* or it's for a group
*/
if (link_specified)
return -ENODEV;
if (tb[IFLA_GROUP])
return rtnl_group_changelink(skb, net, return rtnl_group_changelink(skb, net,
nla_get_u32(tb[IFLA_GROUP]), nla_get_u32(tb[IFLA_GROUP]),
ifm, extack, tb); ifm, extack, tb);
return -ENODEV; return -EINVAL;
} }
if (tb[IFLA_MAP] || tb[IFLA_PROTINFO]) if (tb[IFLA_MAP] || tb[IFLA_PROTINFO])
...@@ -3482,7 +3481,9 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3482,7 +3481,9 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!ops->alloc && !ops->setup) if (!ops->alloc && !ops->setup)
return -EOPNOTSUPP; return -EOPNOTSUPP;
if (!ifname[0]) { if (tb[IFLA_IFNAME]) {
nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
} else {
snprintf(ifname, IFNAMSIZ, "%s%%d", ops->kind); snprintf(ifname, IFNAMSIZ, "%s%%d", ops->kind);
name_assign_type = NET_NAME_ENUM; name_assign_type = NET_NAME_ENUM;
} }
...@@ -3654,8 +3655,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3654,8 +3655,7 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0) if (ifm->ifi_index > 0)
dev = __dev_get_by_index(tgt_net, ifm->ifi_index); dev = __dev_get_by_index(tgt_net, ifm->ifi_index);
else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
dev = rtnl_dev_get(tgt_net, tb[IFLA_IFNAME], dev = rtnl_dev_get(tgt_net, tb);
tb[IFLA_ALT_IFNAME], NULL);
else else
goto out; goto out;
...@@ -3750,8 +3750,7 @@ static int rtnl_linkprop(int cmd, struct sk_buff *skb, struct nlmsghdr *nlh, ...@@ -3750,8 +3750,7 @@ static int rtnl_linkprop(int cmd, struct sk_buff *skb, struct nlmsghdr *nlh,
if (ifm->ifi_index > 0) if (ifm->ifi_index > 0)
dev = __dev_get_by_index(net, ifm->ifi_index); dev = __dev_get_by_index(net, ifm->ifi_index);
else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME]) else if (tb[IFLA_IFNAME] || tb[IFLA_ALT_IFNAME])
dev = rtnl_dev_get(net, tb[IFLA_IFNAME], dev = rtnl_dev_get(net, tb);
tb[IFLA_ALT_IFNAME], NULL);
else else
return -EINVAL; return -EINVAL;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment