Skip to content

Commit af4ae16

Browse files
edumazetshaoyingxu
authored andcommitted
net: do not delay dst_entries_add() in dst_release()
commit ac888d5 upstream. dst_entries_add() uses per-cpu data that might be freed at netns dismantle from ip6_route_net_exit() calling dst_entries_destroy() Before ip6_route_net_exit() can be called, we release all the dsts associated with this netns, via calls to dst_release(), which waits an rcu grace period before calling dst_destroy() dst_entries_add() use in dst_destroy() is racy, because dst_entries_destroy() could have been called already. Decrementing the number of dsts must happen sooner. Notes: 1) in CONFIG_XFRM case, dst_destroy() can call dst_release_immediate(child), this might also cause UAF if the child does not have DST_NOCOUNT set. IPSEC maintainers might take a look and see how to address this. 2) There is also discussion about removing this count of dst, which might happen in future kernels. Fixes: f886497 ("ipv4: fix dst race in sk_dst_get()") Closes: https://lore.kernel.org/lkml/CANn89iLCCGsP7SFn9HKpvnKu96Td4KD08xf7aGtiYgZnkjaL=w@mail.gmail.com/T/ Reported-by: Naresh Kamboju <[email protected]> Tested-by: Linux Kernel Functional Testing <[email protected]> Tested-by: Naresh Kamboju <[email protected]> Signed-off-by: Eric Dumazet <[email protected]> Cc: Xin Long <[email protected]> Cc: Steffen Klassert <[email protected]> Reviewed-by: Xin Long <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Paolo Abeni <[email protected]> [[email protected]: fix merge conflicts] Signed-off-by: Pratyush Yadav <[email protected]>
1 parent be50598 commit af4ae16

File tree

1 file changed

+12
-5
lines changed

1 file changed

+12
-5
lines changed

net/core/dst.c

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,6 @@ struct dst_entry *dst_destroy(struct dst_entry * dst)
109109
child = xdst->child;
110110
}
111111
#endif
112-
if (!(dst->flags & DST_NOCOUNT))
113-
dst_entries_add(dst->ops, -1);
114-
115112
if (dst->ops->destroy)
116113
dst->ops->destroy(dst);
117114
if (dst->dev)
@@ -162,6 +159,12 @@ void dst_dev_put(struct dst_entry *dst)
162159
}
163160
EXPORT_SYMBOL(dst_dev_put);
164161

162+
static void dst_count_dec(struct dst_entry *dst)
163+
{
164+
if (!(dst->flags & DST_NOCOUNT))
165+
dst_entries_add(dst->ops, -1);
166+
}
167+
165168
void dst_release(struct dst_entry *dst)
166169
{
167170
if (dst) {
@@ -171,8 +174,10 @@ void dst_release(struct dst_entry *dst)
171174
if (WARN_ONCE(newrefcnt < 0, "dst_release underflow"))
172175
net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
173176
__func__, dst, newrefcnt);
174-
if (!newrefcnt)
177+
if (!newrefcnt) {
178+
dst_count_dec(dst);
175179
call_rcu(&dst->rcu_head, dst_destroy_rcu);
180+
}
176181
}
177182
}
178183
EXPORT_SYMBOL(dst_release);
@@ -186,8 +191,10 @@ void dst_release_immediate(struct dst_entry *dst)
186191
if (WARN_ONCE(newrefcnt < 0, "dst_release_immediate underflow"))
187192
net_warn_ratelimited("%s: dst:%p refcnt:%d\n",
188193
__func__, dst, newrefcnt);
189-
if (!newrefcnt)
194+
if (!newrefcnt) {
195+
dst_count_dec(dst);
190196
dst_destroy(dst);
197+
}
191198
}
192199
}
193200
EXPORT_SYMBOL(dst_release_immediate);

0 commit comments

Comments
 (0)