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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.