| Message ID | 20250326173634.31096-1-atenart@kernel.org (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: decrease cached dst counters in dst_release | expand |
On 3/26/25 6:36 PM, Antoine Tenart wrote: > Upstream fix ac888d58869b ("net: do not delay dst_entries_add() in > dst_release()") moved decrementing the dst count from dst_destroy to > dst_release to avoid accessing already freed data in case of netns > dismantle. However in case CONFIG_DST_CACHE is enabled and OvS+tunnels > are used, this fix is incomplete as the same issue will be seen for > cached dsts: > > Unable to handle kernel paging request at virtual address ffff5aabf6b5c000 > Call trace: > percpu_counter_add_batch+0x3c/0x160 (P) > dst_release+0xec/0x108 > dst_cache_destroy+0x68/0xd8 > dst_destroy+0x13c/0x168 > dst_destroy_rcu+0x1c/0xb0 > rcu_do_batch+0x18c/0x7d0 > rcu_core+0x174/0x378 > rcu_core_si+0x18/0x30 > > Fix this by invalidating the cache, and thus decrementing cached dst > counters, in dst_release too. > > Fixes: d71785ffc7e7 ("net: add dst_cache to ovs vxlan lwtunnel") > Signed-off-by: Antoine Tenart <atenart@kernel.org> > --- > net/core/dst.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/net/core/dst.c b/net/core/dst.c > index 9552a90d4772..6d76b799ce64 100644 > --- a/net/core/dst.c > +++ b/net/core/dst.c > @@ -165,6 +165,14 @@ static void dst_count_dec(struct dst_entry *dst) > void dst_release(struct dst_entry *dst) > { > if (dst && rcuref_put(&dst->__rcuref)) { > +#ifdef CONFIG_DST_CACHE > + if (dst->flags & DST_METADATA) { > + struct metadata_dst *md_dst = (struct metadata_dst *)dst; > + > + if (md_dst->type == METADATA_IP_TUNNEL) > + dst_cache_reset_now(&md_dst->u.tun_info.dst_cache); I think the fix is correct, but I'm wondering if we have a similar issue for the METADATA_XFRM meta-dst. Isn't: dst_release(md_dst->u.xfrm_info.dst_orig); in metadata_dst_free() going to cause the same UaF? Why don't we need to clean such dst here, too? Thanks! Paolo
diff --git a/net/core/dst.c b/net/core/dst.c index 9552a90d4772..6d76b799ce64 100644 --- a/net/core/dst.c +++ b/net/core/dst.c @@ -165,6 +165,14 @@ static void dst_count_dec(struct dst_entry *dst) void dst_release(struct dst_entry *dst) { if (dst && rcuref_put(&dst->__rcuref)) { +#ifdef CONFIG_DST_CACHE + if (dst->flags & DST_METADATA) { + struct metadata_dst *md_dst = (struct metadata_dst *)dst; + + if (md_dst->type == METADATA_IP_TUNNEL) + dst_cache_reset_now(&md_dst->u.tun_info.dst_cache); + } +#endif dst_count_dec(dst); call_rcu_hurry(&dst->rcu_head, dst_destroy_rcu); }
Upstream fix ac888d58869b ("net: do not delay dst_entries_add() in dst_release()") moved decrementing the dst count from dst_destroy to dst_release to avoid accessing already freed data in case of netns dismantle. However in case CONFIG_DST_CACHE is enabled and OvS+tunnels are used, this fix is incomplete as the same issue will be seen for cached dsts: Unable to handle kernel paging request at virtual address ffff5aabf6b5c000 Call trace: percpu_counter_add_batch+0x3c/0x160 (P) dst_release+0xec/0x108 dst_cache_destroy+0x68/0xd8 dst_destroy+0x13c/0x168 dst_destroy_rcu+0x1c/0xb0 rcu_do_batch+0x18c/0x7d0 rcu_core+0x174/0x378 rcu_core_si+0x18/0x30 Fix this by invalidating the cache, and thus decrementing cached dst counters, in dst_release too. Fixes: d71785ffc7e7 ("net: add dst_cache to ovs vxlan lwtunnel") Signed-off-by: Antoine Tenart <atenart@kernel.org> --- net/core/dst.c | 8 ++++++++ 1 file changed, 8 insertions(+)