From c92bf26ccebc8501863b38c4a4f65b8fef28ce5e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 29 Apr 2022 16:55:06 -0700 Subject: [PATCH 1/3] rtnl: allocate more attr tables on the heap Commit a293974590cf ("rtnetlink: avoid frame size warning in rtnl_newlink()") moved to allocating the largest attribute array of rtnl_newlink() on the heap. Kalle reports the stack has grown above 1k again: net/core/rtnetlink.c:3557:1: error: the frame size of 1104 bytes is larger than 1024 bytes [-Werror=frame-larger-than=] Move more attrs to the heap, wrap them in a struct. Don't bother with linkinfo, it's referenced a lot and we take its size so it's awkward to move, plus it's small (6 elements). Reported-by: Kalle Valo Signed-off-by: Jakub Kicinski Tested-by: Kalle Valo Signed-off-by: Paolo Abeni --- net/core/rtnetlink.c | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 73f2cbc440c9..33919fd5c202 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3302,17 +3302,23 @@ static int rtnl_group_changelink(const struct sk_buff *skb, return 0; } -static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, - struct nlattr **attr, struct netlink_ext_ack *extack) -{ +struct rtnl_newlink_tbs { + struct nlattr *tb[IFLA_MAX + 1]; + struct nlattr *attr[RTNL_MAX_TYPE + 1]; struct nlattr *slave_attr[RTNL_SLAVE_MAX_TYPE + 1]; +}; + +static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, + struct rtnl_newlink_tbs *tbs, + struct netlink_ext_ack *extack) +{ unsigned char name_assign_type = NET_NAME_USER; struct nlattr *linkinfo[IFLA_INFO_MAX + 1]; + struct nlattr ** const tb = tbs->tb; const struct rtnl_link_ops *m_ops; struct net_device *master_dev; struct net *net = sock_net(skb->sk); const struct rtnl_link_ops *ops; - struct nlattr *tb[IFLA_MAX + 1]; struct net *dest_net, *link_net; struct nlattr **slave_data; char kind[MODULE_NAME_LEN]; @@ -3382,12 +3388,12 @@ replay: return -EINVAL; if (ops->maxtype && linkinfo[IFLA_INFO_DATA]) { - err = nla_parse_nested_deprecated(attr, ops->maxtype, + err = nla_parse_nested_deprecated(tbs->attr, ops->maxtype, linkinfo[IFLA_INFO_DATA], ops->policy, extack); if (err < 0) return err; - data = attr; + data = tbs->attr; } if (ops->validate) { err = ops->validate(tb, data, extack); @@ -3403,14 +3409,14 @@ replay: if (m_ops->slave_maxtype && linkinfo[IFLA_INFO_SLAVE_DATA]) { - err = nla_parse_nested_deprecated(slave_attr, + err = nla_parse_nested_deprecated(tbs->slave_attr, m_ops->slave_maxtype, linkinfo[IFLA_INFO_SLAVE_DATA], m_ops->slave_policy, extack); if (err < 0) return err; - slave_data = slave_attr; + slave_data = tbs->slave_attr; } } @@ -3559,15 +3565,15 @@ out_unregister: static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, struct netlink_ext_ack *extack) { - struct nlattr **attr; + struct rtnl_newlink_tbs *tbs; int ret; - attr = kmalloc_array(RTNL_MAX_TYPE + 1, sizeof(*attr), GFP_KERNEL); - if (!attr) + tbs = kmalloc(sizeof(*tbs), GFP_KERNEL); + if (!tbs) return -ENOMEM; - ret = __rtnl_newlink(skb, nlh, attr, extack); - kfree(attr); + ret = __rtnl_newlink(skb, nlh, tbs, extack); + kfree(tbs); return ret; } From 63105e83987a08f73accfba78602a073bf219b69 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 29 Apr 2022 16:55:07 -0700 Subject: [PATCH 2/3] rtnl: split __rtnl_newlink() into two functions __rtnl_newlink() is 250LoC, but has a few clear sections. Move the part which creates a new netdev to a separate function. For ease of review code will be moved in the next change. Signed-off-by: Jakub Kicinski Signed-off-by: Paolo Abeni --- net/core/rtnetlink.c | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 33919fd5c202..1deef11c6b4d 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3302,6 +3302,11 @@ static int rtnl_group_changelink(const struct sk_buff *skb, return 0; } +static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm, + const struct rtnl_link_ops *ops, + struct nlattr **tb, struct nlattr **data, + struct netlink_ext_ack *extack); + struct rtnl_newlink_tbs { struct nlattr *tb[IFLA_MAX + 1]; struct nlattr *attr[RTNL_MAX_TYPE + 1]; @@ -3312,19 +3317,16 @@ static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, struct rtnl_newlink_tbs *tbs, struct netlink_ext_ack *extack) { - unsigned char name_assign_type = NET_NAME_USER; struct nlattr *linkinfo[IFLA_INFO_MAX + 1]; struct nlattr ** const tb = tbs->tb; const struct rtnl_link_ops *m_ops; struct net_device *master_dev; struct net *net = sock_net(skb->sk); const struct rtnl_link_ops *ops; - struct net *dest_net, *link_net; struct nlattr **slave_data; char kind[MODULE_NAME_LEN]; struct net_device *dev; struct ifinfomsg *ifm; - char ifname[IFNAMSIZ]; struct nlattr **data; bool link_specified; int err; @@ -3484,6 +3486,21 @@ replay: return -EOPNOTSUPP; } + return rtnl_newlink_create(skb, ifm, ops, tb, data, extack); +} + +static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm, + const struct rtnl_link_ops *ops, + struct nlattr **tb, struct nlattr **data, + struct netlink_ext_ack *extack) +{ + unsigned char name_assign_type = NET_NAME_USER; + struct net *net = sock_net(skb->sk); + struct net *dest_net, *link_net; + struct net_device *dev; + char ifname[IFNAMSIZ]; + int err; + if (!ops->alloc && !ops->setup) return -EOPNOTSUPP; From 02839cc8d72b49c1440b5019b3f40ca49c4824e5 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Fri, 29 Apr 2022 16:55:08 -0700 Subject: [PATCH 3/3] rtnl: move rtnl_newlink_create() Pure code move. Signed-off-by: Jakub Kicinski Signed-off-by: Paolo Abeni --- net/core/rtnetlink.c | 177 +++++++++++++++++++++---------------------- 1 file changed, 86 insertions(+), 91 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 1deef11c6b4d..eea5ed09e1bb 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3305,7 +3305,92 @@ static int rtnl_group_changelink(const struct sk_buff *skb, static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm, const struct rtnl_link_ops *ops, struct nlattr **tb, struct nlattr **data, - struct netlink_ext_ack *extack); + struct netlink_ext_ack *extack) +{ + unsigned char name_assign_type = NET_NAME_USER; + struct net *net = sock_net(skb->sk); + struct net *dest_net, *link_net; + struct net_device *dev; + char ifname[IFNAMSIZ]; + int err; + + if (!ops->alloc && !ops->setup) + return -EOPNOTSUPP; + + if (tb[IFLA_IFNAME]) { + nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ); + } else { + snprintf(ifname, IFNAMSIZ, "%s%%d", ops->kind); + name_assign_type = NET_NAME_ENUM; + } + + dest_net = rtnl_link_get_net_capable(skb, net, tb, CAP_NET_ADMIN); + if (IS_ERR(dest_net)) + return PTR_ERR(dest_net); + + if (tb[IFLA_LINK_NETNSID]) { + int id = nla_get_s32(tb[IFLA_LINK_NETNSID]); + + link_net = get_net_ns_by_id(dest_net, id); + if (!link_net) { + NL_SET_ERR_MSG(extack, "Unknown network namespace id"); + err = -EINVAL; + goto out; + } + err = -EPERM; + if (!netlink_ns_capable(skb, link_net->user_ns, CAP_NET_ADMIN)) + goto out; + } else { + link_net = NULL; + } + + dev = rtnl_create_link(link_net ? : dest_net, ifname, + name_assign_type, ops, tb, extack); + if (IS_ERR(dev)) { + err = PTR_ERR(dev); + goto out; + } + + dev->ifindex = ifm->ifi_index; + + if (ops->newlink) + err = ops->newlink(link_net ? : net, dev, tb, data, extack); + else + err = register_netdevice(dev); + if (err < 0) { + free_netdev(dev); + goto out; + } + + err = rtnl_configure_link(dev, ifm); + if (err < 0) + goto out_unregister; + if (link_net) { + err = dev_change_net_namespace(dev, dest_net, ifname); + if (err < 0) + goto out_unregister; + } + if (tb[IFLA_MASTER]) { + err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]), extack); + if (err) + goto out_unregister; + } +out: + if (link_net) + put_net(link_net); + put_net(dest_net); + return err; +out_unregister: + if (ops->newlink) { + LIST_HEAD(list_kill); + + ops->dellink(dev, &list_kill); + unregister_netdevice_many(&list_kill); + } else { + unregister_netdevice(dev); + } + goto out; +} struct rtnl_newlink_tbs { struct nlattr *tb[IFLA_MAX + 1]; @@ -3489,96 +3574,6 @@ replay: return rtnl_newlink_create(skb, ifm, ops, tb, data, extack); } -static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm, - const struct rtnl_link_ops *ops, - struct nlattr **tb, struct nlattr **data, - struct netlink_ext_ack *extack) -{ - unsigned char name_assign_type = NET_NAME_USER; - struct net *net = sock_net(skb->sk); - struct net *dest_net, *link_net; - struct net_device *dev; - char ifname[IFNAMSIZ]; - int err; - - if (!ops->alloc && !ops->setup) - return -EOPNOTSUPP; - - if (tb[IFLA_IFNAME]) { - nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ); - } else { - snprintf(ifname, IFNAMSIZ, "%s%%d", ops->kind); - name_assign_type = NET_NAME_ENUM; - } - - dest_net = rtnl_link_get_net_capable(skb, net, tb, CAP_NET_ADMIN); - if (IS_ERR(dest_net)) - return PTR_ERR(dest_net); - - if (tb[IFLA_LINK_NETNSID]) { - int id = nla_get_s32(tb[IFLA_LINK_NETNSID]); - - link_net = get_net_ns_by_id(dest_net, id); - if (!link_net) { - NL_SET_ERR_MSG(extack, "Unknown network namespace id"); - err = -EINVAL; - goto out; - } - err = -EPERM; - if (!netlink_ns_capable(skb, link_net->user_ns, CAP_NET_ADMIN)) - goto out; - } else { - link_net = NULL; - } - - dev = rtnl_create_link(link_net ? : dest_net, ifname, - name_assign_type, ops, tb, extack); - if (IS_ERR(dev)) { - err = PTR_ERR(dev); - goto out; - } - - dev->ifindex = ifm->ifi_index; - - if (ops->newlink) - err = ops->newlink(link_net ? : net, dev, tb, data, extack); - else - err = register_netdevice(dev); - if (err < 0) { - free_netdev(dev); - goto out; - } - - err = rtnl_configure_link(dev, ifm); - if (err < 0) - goto out_unregister; - if (link_net) { - err = dev_change_net_namespace(dev, dest_net, ifname); - if (err < 0) - goto out_unregister; - } - if (tb[IFLA_MASTER]) { - err = do_set_master(dev, nla_get_u32(tb[IFLA_MASTER]), extack); - if (err) - goto out_unregister; - } -out: - if (link_net) - put_net(link_net); - put_net(dest_net); - return err; -out_unregister: - if (ops->newlink) { - LIST_HEAD(list_kill); - - ops->dellink(dev, &list_kill); - unregister_netdevice_many(&list_kill); - } else { - unregister_netdevice(dev); - } - goto out; -} - static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, struct netlink_ext_ack *extack) {