Found while working on the two producers that steer a packet to another device's neighbour, fixed later in this series. Both release the route dst on the way, and a route installed with an encapsulation carries the parameters a device downstream needs: an ip or ip6 encapsulation holds an ip_tunnel_info, where skb_tunnel_info() reads it, and an xfrm encapsulation holds an xfrm_md_info, where skb_xfrm_md_info() reads it. The release discards them, so a tunnel or an xfrm interface downstream has nothing left to build the outer header with, or to select its if_id. The Fix: Add skb_dst_drop_keep_metadata(), which moves those parameters onto a metadata dst of its own before the route is released. The ip and ip6 payload is copied field by field; the route's dst cache belongs to its lwtstate and is shared by every packet that uses the route, so it is not duplicated: the copy starts with an empty cache, which costs the tunnel one route lookup. The xfrm payload keeps the route itself, because the xfrm lookup reads its family; the counted reference the skb already holds moves into the metadata dst, which releases it once it has been consumed. The helper owns the whole operation: a dst that is not a route is not dropped, because a metadata dst is the encapsulation itself. Its callers must not transmit with the stale route, so a failed copy is reported and the skb is left untouched for its owner to drop. Conditions to recreate the bug: Install a route with an encapsulation (ip route add ... encap ip id dst , or encap xfrm if_id ), steer a packet to another device's neighbour with a netdev egress fwd rule or bpf_redirect_neigh(), and release the route dst before neigh_xmit() hands the packet on. A tunnel or xfrm device in the path then finds no parameters in skb_tunnel_info() or skb_xfrm_md_info() and drops the packet instead of using them. Reviewed-by: Victor Nogueira Signed-off-by: Jamal Hadi Salim --- include/net/dst_metadata.h | 1 + net/core/dst.c | 72 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/include/net/dst_metadata.h b/include/net/dst_metadata.h index f45d1e3163f0..32d74b4f009f 100644 --- a/include/net/dst_metadata.h +++ b/include/net/dst_metadata.h @@ -136,6 +136,7 @@ struct metadata_dst *metadata_dst_alloc(u8 optslen, enum metadata_type type, void metadata_dst_free_percpu(struct metadata_dst __percpu *md_dst); struct metadata_dst __percpu * metadata_dst_alloc_percpu(u8 optslen, enum metadata_type type, gfp_t flags); +bool skb_dst_drop_keep_metadata(struct sk_buff *skb); static inline struct metadata_dst *tun_rx_dst(int md_size) { diff --git a/net/core/dst.c b/net/core/dst.c index 092861133023..daa003979d59 100644 --- a/net/core/dst.c +++ b/net/core/dst.c @@ -353,3 +353,75 @@ void metadata_dst_free_percpu(struct metadata_dst __percpu *md_dst) free_percpu(md_dst); } EXPORT_SYMBOL_GPL(metadata_dst_free_percpu); + +/** + * skb_dst_drop_keep_metadata - release a route dst, keeping its LWT metadata + * @skb: buffer whose dst is to be released + * + * A route installed with an encapsulation carries the parameters a downstream + * device needs in dst->lwtstate, where skb_tunnel_info() and skb_xfrm_md_info() + * find them. Releasing such a route would discard them, so move them onto a + * metadata dst of their own first. The route is not duplicated: the copy + * starts with an empty cache, and for xfrm the route is kept because the + * lookup needs its family. + * + * A dst that is metadata is left in place. False means no copy could be + * allocated and the caller must drop the skb. + */ +bool skb_dst_drop_keep_metadata(struct sk_buff *skb) +{ + struct dst_entry *old = skb_dst(skb); + struct metadata_dst *md_dst = NULL; + + if (!skb_valid_dst(skb)) + return true; + + if (old->lwtstate && old->lwtstate->type == LWTUNNEL_ENCAP_XFRM) { + struct xfrm_md_info *src = lwt_xfrm_info(old->lwtstate); + + md_dst = metadata_dst_alloc(0, METADATA_XFRM, GFP_ATOMIC); + if (!md_dst) + return false; + + /* Count the route so it outlives the skb, then move the + * reference into the metadata dst. + */ + if (!skb_dst_force(skb)) { + metadata_dst_free(md_dst); + return false; + } + + md_dst->u.xfrm_info.if_id = src->if_id; + md_dst->u.xfrm_info.link = src->link; + md_dst->u.xfrm_info.dst_orig = old; + skb_dstref_steal(skb); + skb_dst_set(skb, &md_dst->dst); + return true; + } + + if (old->lwtstate && + (old->lwtstate->type == LWTUNNEL_ENCAP_IP || + old->lwtstate->type == LWTUNNEL_ENCAP_IP6)) { + struct ip_tunnel_info *src = lwt_tun_info(old->lwtstate); + struct ip_tunnel_info *dst; + + md_dst = metadata_dst_alloc(src->options_len, + METADATA_IP_TUNNEL, GFP_ATOMIC); + if (!md_dst) + return false; + + dst = &md_dst->u.tun_info; + dst->key = src->key; + dst->encap = src->encap; + dst->options_len = src->options_len; + dst->mode = src->mode; + memcpy(ip_tunnel_info_opts(dst), ip_tunnel_info_opts(src), + src->options_len); + } + + skb_dst_drop(skb); + if (md_dst) + skb_dst_set(skb, &md_dst->dst); + return true; +} +EXPORT_SYMBOL_GPL(skb_dst_drop_keep_metadata); -- 2.43.0