diff mbox series

[net-next] rtnetlink: move validate_linkmsg out of do_setlink

Message ID cf2ef061e08251faf9e8be25ff0d61150c030475.1686585334.git.lucien.xin@gmail.com (mailing list archive)
State Accepted
Commit 89da780aa4c7667c8b1a144959206262aebdeabe
Delegated to: Netdev Maintainers
Headers show
Series [net-next] rtnetlink: move validate_linkmsg out of do_setlink | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 10 this patch: 10
netdev/cc_maintainers warning 2 maintainers not CCed: razor@blackwall.org idosch@nvidia.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 10 this patch: 10
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 125 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Xin Long June 12, 2023, 3:55 p.m. UTC 
This patch moves validate_linkmsg() out of do_setlink() to its callers
and deletes the early validate_linkmsg() call in __rtnl_newlink(), so
that it will not call validate_linkmsg() twice in either of the paths:

  - __rtnl_newlink() -> do_setlink()
  - __rtnl_newlink() -> rtnl_newlink_create() -> rtnl_create_link()

Additionally, as validate_linkmsg() is now only called with a real
dev, we can remove the NULL check for dev in validate_linkmsg().

Note that we moved validate_linkmsg() check to the places where it has
not done any changes to the dev, as Jakub suggested.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/core/rtnetlink.c | 83 ++++++++++++++++++++++----------------------
 1 file changed, 42 insertions(+), 41 deletions(-)

Comments

Simon Horman June 14, 2023, 11:49 a.m. UTC | #1 
On Mon, Jun 12, 2023 at 11:55:34AM -0400, Xin Long wrote:
> This patch moves validate_linkmsg() out of do_setlink() to its callers
> and deletes the early validate_linkmsg() call in __rtnl_newlink(), so
> that it will not call validate_linkmsg() twice in either of the paths:
> 
>   - __rtnl_newlink() -> do_setlink()
>   - __rtnl_newlink() -> rtnl_newlink_create() -> rtnl_create_link()
> 
> Additionally, as validate_linkmsg() is now only called with a real
> dev, we can remove the NULL check for dev in validate_linkmsg().
> 
> Note that we moved validate_linkmsg() check to the places where it has
> not done any changes to the dev, as Jakub suggested.
> 
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
patchwork-bot+netdevbpf@kernel.org June 15, 2023, 5:50 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 12 Jun 2023 11:55:34 -0400 you wrote:
> This patch moves validate_linkmsg() out of do_setlink() to its callers
> and deletes the early validate_linkmsg() call in __rtnl_newlink(), so
> that it will not call validate_linkmsg() twice in either of the paths:
> 
>   - __rtnl_newlink() -> do_setlink()
>   - __rtnl_newlink() -> rtnl_newlink_create() -> rtnl_create_link()
> 
> [...]

Here is the summary with links:
  - [net-next] rtnetlink: move validate_linkmsg out of do_setlink
    https://git.kernel.org/netdev/net-next/c/89da780aa4c7

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 41de3a2f29e1..b9824708c3bd 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2377,45 +2377,43 @@  static	int rtnl_set_vf_rate(struct net_device *dev, int vf, int min_tx_rate,
 static int validate_linkmsg(struct net_device *dev, struct nlattr *tb[],
 			    struct netlink_ext_ack *extack)
 {
-	if (dev) {
-		if (tb[IFLA_ADDRESS] &&
-		    nla_len(tb[IFLA_ADDRESS]) < dev->addr_len)
-			return -EINVAL;
+	if (tb[IFLA_ADDRESS] &&
+	    nla_len(tb[IFLA_ADDRESS]) < dev->addr_len)
+		return -EINVAL;
 
-		if (tb[IFLA_BROADCAST] &&
-		    nla_len(tb[IFLA_BROADCAST]) < dev->addr_len)
-			return -EINVAL;
+	if (tb[IFLA_BROADCAST] &&
+	    nla_len(tb[IFLA_BROADCAST]) < dev->addr_len)
+		return -EINVAL;
 
-		if (tb[IFLA_GSO_MAX_SIZE] &&
-		    nla_get_u32(tb[IFLA_GSO_MAX_SIZE]) > dev->tso_max_size) {
-			NL_SET_ERR_MSG(extack, "too big gso_max_size");
-			return -EINVAL;
-		}
+	if (tb[IFLA_GSO_MAX_SIZE] &&
+	    nla_get_u32(tb[IFLA_GSO_MAX_SIZE]) > dev->tso_max_size) {
+		NL_SET_ERR_MSG(extack, "too big gso_max_size");
+		return -EINVAL;
+	}
 
-		if (tb[IFLA_GSO_MAX_SEGS] &&
-		    (nla_get_u32(tb[IFLA_GSO_MAX_SEGS]) > GSO_MAX_SEGS ||
-		     nla_get_u32(tb[IFLA_GSO_MAX_SEGS]) > dev->tso_max_segs)) {
-			NL_SET_ERR_MSG(extack, "too big gso_max_segs");
-			return -EINVAL;
-		}
+	if (tb[IFLA_GSO_MAX_SEGS] &&
+	    (nla_get_u32(tb[IFLA_GSO_MAX_SEGS]) > GSO_MAX_SEGS ||
+	     nla_get_u32(tb[IFLA_GSO_MAX_SEGS]) > dev->tso_max_segs)) {
+		NL_SET_ERR_MSG(extack, "too big gso_max_segs");
+		return -EINVAL;
+	}
 
-		if (tb[IFLA_GRO_MAX_SIZE] &&
-		    nla_get_u32(tb[IFLA_GRO_MAX_SIZE]) > GRO_MAX_SIZE) {
-			NL_SET_ERR_MSG(extack, "too big gro_max_size");
-			return -EINVAL;
-		}
+	if (tb[IFLA_GRO_MAX_SIZE] &&
+	    nla_get_u32(tb[IFLA_GRO_MAX_SIZE]) > GRO_MAX_SIZE) {
+		NL_SET_ERR_MSG(extack, "too big gro_max_size");
+		return -EINVAL;
+	}
 
-		if (tb[IFLA_GSO_IPV4_MAX_SIZE] &&
-		    nla_get_u32(tb[IFLA_GSO_IPV4_MAX_SIZE]) > dev->tso_max_size) {
-			NL_SET_ERR_MSG(extack, "too big gso_ipv4_max_size");
-			return -EINVAL;
-		}
+	if (tb[IFLA_GSO_IPV4_MAX_SIZE] &&
+	    nla_get_u32(tb[IFLA_GSO_IPV4_MAX_SIZE]) > dev->tso_max_size) {
+		NL_SET_ERR_MSG(extack, "too big gso_ipv4_max_size");
+		return -EINVAL;
+	}
 
-		if (tb[IFLA_GRO_IPV4_MAX_SIZE] &&
-		    nla_get_u32(tb[IFLA_GRO_IPV4_MAX_SIZE]) > GRO_MAX_SIZE) {
-			NL_SET_ERR_MSG(extack, "too big gro_ipv4_max_size");
-			return -EINVAL;
-		}
+	if (tb[IFLA_GRO_IPV4_MAX_SIZE] &&
+	    nla_get_u32(tb[IFLA_GRO_IPV4_MAX_SIZE]) > GRO_MAX_SIZE) {
+		NL_SET_ERR_MSG(extack, "too big gro_ipv4_max_size");
+		return -EINVAL;
 	}
 
 	if (tb[IFLA_AF_SPEC]) {
@@ -2736,10 +2734,6 @@  static int do_setlink(const struct sk_buff *skb,
 	char ifname[IFNAMSIZ];
 	int err;
 
-	err = validate_linkmsg(dev, tb, extack);
-	if (err < 0)
-		return err;
-
 	if (tb[IFLA_IFNAME])
 		nla_strscpy(ifname, tb[IFLA_IFNAME], IFNAMSIZ);
 	else
@@ -3156,6 +3150,10 @@  static int rtnl_setlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto errout;
 	}
 
+	err = validate_linkmsg(dev, tb, extack);
+	if (err < 0)
+		goto errout;
+
 	err = do_setlink(skb, dev, ifm, extack, tb, 0);
 errout:
 	return err;
@@ -3399,6 +3397,9 @@  static int rtnl_group_changelink(const struct sk_buff *skb,
 
 	for_each_netdev_safe(net, dev, aux) {
 		if (dev->group == group) {
+			err = validate_linkmsg(dev, tb, extack);
+			if (err < 0)
+				return err;
 			err = do_setlink(skb, dev, ifm, extack, tb, 0);
 			if (err < 0)
 				return err;
@@ -3556,10 +3557,6 @@  static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 			m_ops = master_dev->rtnl_link_ops;
 	}
 
-	err = validate_linkmsg(dev, tb, extack);
-	if (err < 0)
-		return err;
-
 	if (tb[IFLA_LINKINFO]) {
 		err = nla_parse_nested_deprecated(linkinfo, IFLA_INFO_MAX,
 						  tb[IFLA_LINKINFO],
@@ -3623,6 +3620,10 @@  static int __rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh,
 		if (nlh->nlmsg_flags & NLM_F_REPLACE)
 			return -EOPNOTSUPP;
 
+		err = validate_linkmsg(dev, tb, extack);
+		if (err < 0)
+			return err;
+
 		if (linkinfo[IFLA_INFO_DATA]) {
 			if (!ops || ops != dev->rtnl_link_ops ||
 			    !ops->changelink)