> The combination of IPIP tunnel with direct xmit, eg. bridge device, > breaks because no dst_entry is provided to check the skb headroom and to > set the iph->frag_off field. This leads to invalid dst usage and can > trigger a crash in the tunnel transmit path. > > Fix this by moving dst_cache and dst_cookie out of the runtime union so > that they can be shared by neighbour, xfrm, and direct tunnel flows. > For FLOW_OFFLOAD_XMIT_DIRECT tuples carrying tunnel metadata, preserve > route state in these shared fields and release it through the common > dst release path. > > Since dst_entry is now available to the three supported xmit modes and > dst_release() already deals with NULL dst, remove the xmit type check > in nft_flow_dst_release(). Moreover, skip the check if the dst entry > is NULL in nf_flow_dst_check() which is now the case for the direct > xmit case. > > Based on patch from Rein Wei . > > Fixes: d30301ba4b07 ("netfilter: flowtable: Add IPIP tx sw acceleration") > Reported-by: Yuan Tan > Reported-by: Xin Liu > Reported-by: Zhengyang Chen > Reported-by: Ren Wei > Signed-off-by: Pablo Neira Ayuso Acked-by: Lorenzo Bianconi > --- > v2: - reorder dst_cookie and dst_cache to fix a 4-byte hole in struct > - remove unnecessary zeroing on dst_cookie and dst_cache, struct is already zeroed at init > Both updates per Lorenzo Bianconi's comment. > > include/net/netfilter/nf_flow_table.h | 5 +++-- > net/netfilter/nf_flow_table_core.c | 12 ++++++++---- > net/netfilter/nf_flow_table_ip.c | 3 +-- > 3 files changed, 12 insertions(+), 8 deletions(-) > > diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h > index dc5c9b48e65a..ce414118962f 100644 > --- a/include/net/netfilter/nf_flow_table.h > +++ b/include/net/netfilter/nf_flow_table.h > @@ -155,11 +155,12 @@ struct flow_offload_tuple { > tun_num:2, > in_vlan_ingress:2; > u16 mtu; > + u32 dst_cookie; > + struct dst_entry *dst_cache; > + > union { > struct { > - struct dst_entry *dst_cache; > u32 ifidx; > - u32 dst_cookie; > }; > struct { > u32 ifidx; > diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c > index d06ce0848b68..2a829b5e8240 100644 > --- a/net/netfilter/nf_flow_table_core.c > +++ b/net/netfilter/nf_flow_table_core.c > @@ -127,12 +127,18 @@ static int flow_offload_fill_route(struct flow_offload *flow, > > switch (route->tuple[dir].xmit_type) { > case FLOW_OFFLOAD_XMIT_DIRECT: > + if (flow_tuple->tun_num) { > + flow_tuple->dst_cache = dst; > + flow_tuple->dst_cookie = > + flow_offload_dst_cookie(flow_tuple); > + } > memcpy(flow_tuple->out.h_dest, route->tuple[dir].out.h_dest, > ETH_ALEN); > memcpy(flow_tuple->out.h_source, route->tuple[dir].out.h_source, > ETH_ALEN); > flow_tuple->out.ifidx = route->tuple[dir].out.ifindex; > - dst_release(dst); > + if (!flow_tuple->tun_num) > + dst_release(dst); > break; > case FLOW_OFFLOAD_XMIT_XFRM: > case FLOW_OFFLOAD_XMIT_NEIGH: > @@ -152,9 +158,7 @@ static int flow_offload_fill_route(struct flow_offload *flow, > static void nft_flow_dst_release(struct flow_offload *flow, > enum flow_offload_tuple_dir dir) > { > - if (flow->tuplehash[dir].tuple.xmit_type == FLOW_OFFLOAD_XMIT_NEIGH || > - flow->tuplehash[dir].tuple.xmit_type == FLOW_OFFLOAD_XMIT_XFRM) > - dst_release(flow->tuplehash[dir].tuple.dst_cache); > + dst_release(flow->tuplehash[dir].tuple.dst_cache); > } > > void flow_offload_route_init(struct flow_offload *flow, > diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c > index 089f2bc19972..0b78decce8a9 100644 > --- a/net/netfilter/nf_flow_table_ip.c > +++ b/net/netfilter/nf_flow_table_ip.c > @@ -299,8 +299,7 @@ static bool nf_flow_exceeds_mtu(const struct sk_buff *skb, unsigned int mtu) > > static inline bool nf_flow_dst_check(struct flow_offload_tuple *tuple) > { > - if (tuple->xmit_type != FLOW_OFFLOAD_XMIT_NEIGH && > - tuple->xmit_type != FLOW_OFFLOAD_XMIT_XFRM) > + if (!tuple->dst_cache) > return true; > > return dst_check(tuple->dst_cache, tuple->dst_cookie); > -- > 2.47.3 >