netdev - Re: [PATCH net-next] ipv4/fib: send RTM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Fri, 4 Aug 2023 16:09:08 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Ido Schimmel <idosch@...sch.org>
Cc: David Ahern <dsahern@...nel.org>,
	Stephen Hemminger <stephen@...workplumber.org>,
	netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Thomas Haller <thaller@...hat.com>
Subject: Re: [PATCH net-next] ipv4/fib: send RTM_DELROUTE notify when flush
 fib

On Sun, Jul 23, 2023 at 10:38:11AM +0300, Ido Schimmel wrote:
> > > >> Will you get a notification in this case for 198.51.100.0/24?
> > > > 
> > > > No. Do you think it is expected with this patch or not?
> > > 
> > > The intent is that notifications are sent for link events but not route
> > > events which are easily deduced from the link events.
> > 
> > Sorry, I didn't get what you mean. The link events should be notified to user
> > via rtmsg_ifinfo_event()? So I think here we ignore the route events caused by
> > link down looks reasonable.
> 
> The route in the scenario I mentioned wasn't deleted because of a link
> event, but because the source address was deleted yet no notification
> was emitted. IMO, this is wrong given the description of the patch.
> 

Hi Ido,

I reconsidered this issue this week. As we can't get the device status in
fib_table_flush(). How about adding another flag to track the deleted src
entries. e.g. RTNH_F_UNRESOLVED. Which is only used in ipmr currently.
When the src route address is deleted, the route entry also could be
considered as unresolved. With this idea, the patch could be like:

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 51c13cf9c5ae..5c41d34ab447 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -420,7 +420,7 @@ struct rtnexthop {
 #define RTNH_F_ONLINK          4       /* Gateway is forced on link    */
 #define RTNH_F_OFFLOAD         8       /* Nexthop is offloaded */
 #define RTNH_F_LINKDOWN                16      /* carrier-down on nexthop */
-#define RTNH_F_UNRESOLVED      32      /* The entry is unresolved (ipmr) */
+#define RTNH_F_UNRESOLVED      32      /* The entry is unresolved (ipmr/dead src) */
 #define RTNH_F_TRAP            64      /* Nexthop is trapping packets */

 #define RTNH_COMPARE_MASK      (RTNH_F_DEAD | RTNH_F_LINKDOWN | \
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 65ba18a91865..a7ef21a6d271 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1883,7 +1883,7 @@ int fib_sync_down_addr(struct net_device *dev, __be32 local)
                    fi->fib_tb_id != tb_id)
                        continue;
                if (fi->fib_prefsrc == local) {
-                       fi->fib_flags |= RTNH_F_DEAD;
+                       fi->fib_flags |= (RTNH_F_DEAD | RTNH_F_UNRESOLVED);
                        ret++;
                }
        }
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 74d403dbd2b4..88c593967063 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -2026,6 +2026,7 @@ void fib_table_flush_external(struct fib_table *tb)
 int fib_table_flush(struct net *net, struct fib_table *tb, bool flush_all)
 {
        struct trie *t = (struct trie *)tb->tb_data;
+       struct nl_info info = { .nl_net = net };
        struct key_vector *pn = t->kv;
        unsigned long cindex = 1;
        struct hlist_node *tmp;
@@ -2088,6 +2089,11 @@ int fib_table_flush(struct net *net, struct fib_table *tb, bool flush_all)

                        fib_notify_alias_delete(net, n->key, &n->leaf, fa,
                                                NULL);
+                       if (fi->fib_flags & RTNH_F_UNRESOLVED) {
+                               fi->fib_flags &= ~RTNH_F_UNRESOLVED;
+                               rtmsg_fib(RTM_DELROUTE, htonl(n->key), fa,
+                                         KEYLENGTH - fa->fa_slen, tb->tb_id, &info, 0);
+                       }
                        hlist_del_rcu(&fa->fa_list);
                        fib_release_info(fa->fa_info);
                        alias_free_mem_rcu(fa);

Thanks
Hangbin