From: thinker.li@gmail.com
To: netdev@vger.kernel.org, martin.lau@linux.dev,
kernel-team@meta.com, davem@davemloft.net, kuba@kernel.org,
pabeni@redhat.com, dsahern@kernel.org, edumazet@google.com
Cc: sinquersw@gmail.com, kuifeng@meta.com,
Kui-Feng Lee <thinker.li@gmail.com>,
syzbot+c15aa445274af8674f41@syzkaller.appspotmail.com
Subject: [PATCH net-next v3 1/2] net/ipv6: insert a f6i to a GC list only if the f6i is in a fib6_table tree.
Date: Wed, 13 Dec 2023 13:37:34 -0800 [thread overview]
Message-ID: <20231213213735.434249-2-thinker.li@gmail.com> (raw)
In-Reply-To: <20231213213735.434249-1-thinker.li@gmail.com>
From: Kui-Feng Lee <thinker.li@gmail.com>
Check f6i->fib6_node and hlist_unhashed(&f6i->gc_link) before inserting a
f6i (fib6_info) to tb6_gc_hlist.
The current implementation checks if f6i->fib6_table is not NULL to
determines if a f6i is on a tree, however it is not enough. When a f6i is
removed from a fib6_table, f6i->fib6_table is not reset. However, fib6_node
is always reset when a f6i is removed from a fib6_table and is set when a
f6i is added to a fib6_table. So, f6i->fib6_node is a reliable way to
determine if a f6i is on a tree.
The current implementation checks RTF_EXPIRES on f6i->fib6_flags to
determine if a f6i is on a GC list. It also consider if the f6i is on a
tree before making a conclusion. This is indirect and complicated. The new
solution is checking hlist_unhashed(&f6i->gc_link), a clear evidence.
Putting them together, these changes provide more reliable signals to
determines if a f6i should be added/or removed to a GC list.
Fixes: 3dec89b14d37 ("net/ipv6: Remove expired routes with a separated list of routes.")
Reported-by: syzbot+c15aa445274af8674f41@syzkaller.appspotmail.com
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: dsahern@kernel.org
---
include/net/ip6_fib.h | 46 ++++++++++++++++++++++++++++++++-----------
net/ipv6/route.c | 6 +++---
2 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 1ba9f4ddf2f6..1213722c394f 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -500,21 +500,47 @@ void fib6_gc_cleanup(void);
int fib6_init(void);
-/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
- * NULL.
- */
-static inline void fib6_set_expires_locked(struct fib6_info *f6i,
- unsigned long expires)
+static inline void fib6_add_gc_list(struct fib6_info *f6i)
{
struct fib6_table *tb6;
tb6 = f6i->fib6_table;
- f6i->expires = expires;
- if (tb6 && !fib6_has_expires(f6i))
+ if (tb6 &&
+ rcu_dereference_protected(f6i->fib6_node,
+ lockdep_is_held(&tb6->tb6_lock)) &&
+ hlist_unhashed(&f6i->gc_link))
hlist_add_head(&f6i->gc_link, &tb6->tb6_gc_hlist);
+}
+
+static inline void fib6_del_gc_list(struct fib6_info *f6i)
+{
+ if (!hlist_unhashed(&f6i->gc_link))
+ hlist_del_init(&f6i->gc_link);
+}
+
+static inline void __fib6_set_expires(struct fib6_info *f6i,
+ unsigned long expires)
+{
+ f6i->expires = expires;
f6i->fib6_flags |= RTF_EXPIRES;
}
+static inline void __fib6_clean_expires(struct fib6_info *f6i)
+{
+ f6i->fib6_flags &= ~RTF_EXPIRES;
+ f6i->expires = 0;
+}
+
+/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
+ * NULL.
+ */
+static inline void fib6_set_expires_locked(struct fib6_info *f6i,
+ unsigned long expires)
+{
+ __fib6_set_expires(f6i, expires);
+ fib6_add_gc_list(f6i);
+}
+
/* fib6_info must be locked by the caller, and fib6_info->fib6_table can be
* NULL. If fib6_table is NULL, the fib6_info will no be inserted into the
* list of GC candidates until it is inserted into a table.
@@ -529,10 +555,8 @@ static inline void fib6_set_expires(struct fib6_info *f6i,
static inline void fib6_clean_expires_locked(struct fib6_info *f6i)
{
- if (fib6_has_expires(f6i))
- hlist_del_init(&f6i->gc_link);
- f6i->fib6_flags &= ~RTF_EXPIRES;
- f6i->expires = 0;
+ fib6_del_gc_list(f6i);
+ __fib6_clean_expires(f6i);
}
static inline void fib6_clean_expires(struct fib6_info *f6i)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b132feae3393..dcaeb88d73aa 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3763,10 +3763,10 @@ static struct fib6_info *ip6_route_info_create(struct fib6_config *cfg,
rt->dst_nocount = true;
if (cfg->fc_flags & RTF_EXPIRES)
- fib6_set_expires_locked(rt, jiffies +
- clock_t_to_jiffies(cfg->fc_expires));
+ __fib6_set_expires(rt, jiffies +
+ clock_t_to_jiffies(cfg->fc_expires));
else
- fib6_clean_expires_locked(rt);
+ __fib6_clean_expires(rt);
if (cfg->fc_protocol == RTPROT_UNSPEC)
cfg->fc_protocol = RTPROT_BOOT;
--
2.34.1
next prev parent reply other threads:[~2023-12-13 21:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-13 21:37 [PATCH net-next v3 0/2] Fix dangling pointer at f6i->gc_link thinker.li
2023-12-13 21:37 ` thinker.li [this message]
2023-12-14 6:11 ` [PATCH net-next v3 1/2] net/ipv6: insert a f6i to a GC list only if the f6i is in a fib6_table tree David Ahern
2023-12-14 23:43 ` Kui-Feng Lee
2023-12-15 19:12 ` Kui-Feng Lee
2023-12-16 18:36 ` David Ahern
2023-12-18 1:05 ` Kui-Feng Lee
2023-12-18 1:16 ` Kui-Feng Lee
2023-12-13 21:37 ` [PATCH net-next v3 2/2] selftests: fib_tests: Add tests for toggling between w/ and w/o expires thinker.li
2023-12-14 3:32 ` Hangbin Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231213213735.434249-2-thinker.li@gmail.com \
--to=thinker.li@gmail.com \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=kernel-team@meta.com \
--cc=kuba@kernel.org \
--cc=kuifeng@meta.com \
--cc=martin.lau@linux.dev \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sinquersw@gmail.com \
--cc=syzbot+c15aa445274af8674f41@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).