Commit cfc56f85 authored by Antoine Tenart's avatar Antoine Tenart Committed by David S. Miller

net: do not keep the dst cache when uncloning an skb dst and its metadata

When uncloning an skb dst and its associated metadata a new dst+metadata
is allocated and the tunnel information from the old metadata is copied
over there.

The issue is the tunnel metadata has references to cached dst, which are
copied along the way. When a dst+metadata refcount drops to 0 the
metadata is freed including the cached dst entries. As they are also
referenced in the initial dst+metadata, this ends up in UaFs.

In practice the above did not happen because of another issue, the
dst+metadata was never freed because its refcount never dropped to 0
(this will be fixed in a subsequent patch).

Fix this by initializing the dst cache after copying the tunnel
information from the old metadata to also unshare the dst cache.

Fixes: d71785ff ("net: add dst_cache to ovs vxlan lwtunnel")
Cc: Paolo Abeni <pabeni@redhat.com>
Reported-by: default avatarVlad Buslov <vladbu@nvidia.com>
Tested-by: default avatarVlad Buslov <vladbu@nvidia.com>
Signed-off-by: default avatarAntoine Tenart <atenart@kernel.org>
Acked-by: default avatarPaolo Abeni <pabeni@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 7db788ad
...@@ -123,6 +123,19 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb) ...@@ -123,6 +123,19 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb)
memcpy(&new_md->u.tun_info, &md_dst->u.tun_info, memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
sizeof(struct ip_tunnel_info) + md_size); sizeof(struct ip_tunnel_info) + md_size);
#ifdef CONFIG_DST_CACHE
/* Unclone the dst cache if there is one */
if (new_md->u.tun_info.dst_cache.cache) {
int ret;
ret = dst_cache_init(&new_md->u.tun_info.dst_cache, GFP_ATOMIC);
if (ret) {
metadata_dst_free(new_md);
return ERR_PTR(ret);
}
}
#endif
skb_dst_drop(skb); skb_dst_drop(skb);
dst_hold(&new_md->dst); dst_hold(&new_md->dst);
skb_dst_set(skb, &new_md->dst); skb_dst_set(skb, &new_md->dst);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment