ipv4: fib: Move fib_valid_key_len() to rtm_to_fib_config().

fib_valid_key_len() is called in the beginning of fib_table_insert()
or fib_table_delete() to check if the prefix length is valid.

fib_table_insert() and fib_table_delete() are called from 3 paths

  - ip_rt_ioctl()
  - inet_rtm_newroute() / inet_rtm_delroute()
  - fib_magic()

In the first ioctl() path, rtentry_to_fib_config() checks the prefix
length with bad_mask().  Also, fib_magic() always passes the correct
prefix: 32 or ifa->ifa_prefixlen, which is already validated.

Let's move fib_valid_key_len() to the rtnetlink path, rtm_to_fib_config().

While at it, 2 direct returns in rtm_to_fib_config() are changed to
goto to match other places in the same function

Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: David Ahern <dsahern@kernel.org>
Link: https://patch.msgid.link/20250228042328.96624-12-kuniyu@amazon.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index a76dacc3e577a53fe045787ec9eaf002a311720b..a6372d934e4594eea8cf31d72a0dfa4827adbb49 100644 (file)
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -835,19 +835,33 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
                }
        }
 
+       if (cfg->fc_dst_len > 32) {
+               NL_SET_ERR_MSG(extack, "Invalid prefix length");
+               err = -EINVAL;
+               goto errout;
+       }
+
+       if (cfg->fc_dst_len < 32 && (ntohl(cfg->fc_dst) << cfg->fc_dst_len)) {
+               NL_SET_ERR_MSG(extack, "Invalid prefix for given prefix length");
+               err = -EINVAL;
+               goto errout;
+       }
+
        if (cfg->fc_nh_id) {
                if (cfg->fc_oif || cfg->fc_gw_family ||
                    cfg->fc_encap || cfg->fc_mp) {
                        NL_SET_ERR_MSG(extack,
                                       "Nexthop specification and nexthop id are mutually exclusive");
-                       return -EINVAL;
+                       err = -EINVAL;
+                       goto errout;
                }
        }
 
        if (has_gw && has_via) {
                NL_SET_ERR_MSG(extack,
                               "Nexthop configuration can not contain both GATEWAY and VIA");
-               return -EINVAL;
+               err = -EINVAL;
+               goto errout;
        }
 
        if (!cfg->fc_table)

diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index d6411ac810961a0b78a5b5c8263e217b69b033e0..59a6f0a9638f99a50f89d0903b16922ebf4d8bae 100644 (file)
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1187,22 +1187,6 @@ static int fib_insert_alias(struct trie *t, struct key_vector *tp,
        return 0;
 }
 
-static bool fib_valid_key_len(u32 key, u8 plen, struct netlink_ext_ack *extack)
-{
-       if (plen > KEYLENGTH) {
-               NL_SET_ERR_MSG(extack, "Invalid prefix length");
-               return false;
-       }
-
-       if ((plen < KEYLENGTH) && (key << plen)) {
-               NL_SET_ERR_MSG(extack,
-                              "Invalid prefix for given prefix length");
-               return false;
-       }
-
-       return true;
-}
-
 static void fib_remove_alias(struct trie *t, struct key_vector *tp,
                             struct key_vector *l, struct fib_alias *old);
 
@@ -1223,9 +1207,6 @@ int fib_table_insert(struct net *net, struct fib_table *tb,
 
        key = ntohl(cfg->fc_dst);
 
-       if (!fib_valid_key_len(key, plen, extack))
-               return -EINVAL;
-
        pr_debug("Insert table=%u %08x/%d\n", tb->tb_id, key, plen);
 
        fi = fib_create_info(cfg, extack);
@@ -1717,9 +1698,6 @@ int fib_table_delete(struct net *net, struct fib_table *tb,
 
        key = ntohl(cfg->fc_dst);
 
-       if (!fib_valid_key_len(key, plen, extack))
-               return -EINVAL;
-
        l = fib_find_node(t, &tp, key);
        if (!l)
                return -ESRCH;