| Message ID | 20191231165654.19434-3-nicolas.dichtel@6wind.com |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | David Miller |
| Headers | show |
| Series | ipsec interfaces: fix sending with bpf_redirect() / AF_PACKET sockets | expand |
On Tue, Dec 31, 2019 at 05:56:54PM +0100, Nicolas Dichtel wrote: > With an ebpf program that redirects packets through a xfrm interface, > packets are dropped because no dst is attached to skb. > > This could also be reproduced with an AF_PACKET socket, with the following > python script (xfrm1 is a xfrm interface): > > import socket > send_s = socket.socket(socket.AF_PACKET, socket.SOCK_RAW, 0) > # scapy > # p = IP(src='10.100.0.2', dst='10.200.0.1')/ICMP(type='echo-request') > # raw(p) > req = b'E\x00\x00\x1c\x00\x01\x00\x00@\x01e\xb2\nd\x00\x02\n\xc8\x00\x01\x08\x00\xf7\xff\x00\x00\x00\x00' > send_s.sendto(req, ('xfrm1', 0x800, 0, 0)) > > It was also not possible to send an ip packet through an AF_PACKET socket > because a LL header was expected. Let's remove those LL header constraints. > > Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> > --- > net/xfrm/xfrm_interface.c | 22 ++++++++++++++++++---- > 1 file changed, 18 insertions(+), 4 deletions(-) > > diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c > index 7ac1542feaf8..55978a1501ec 100644 > --- a/net/xfrm/xfrm_interface.c > +++ b/net/xfrm/xfrm_interface.c > @@ -343,6 +343,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) > { > struct xfrm_if *xi = netdev_priv(dev); > struct net_device_stats *stats = &xi->dev->stats; > + struct dst_entry *dst = skb_dst(skb); > struct flowi fl; > int ret; > > @@ -352,10 +353,26 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) > case htons(ETH_P_IPV6): > xfrm_decode_session(skb, &fl, AF_INET6); > memset(IP6CB(skb), 0, sizeof(*IP6CB(skb))); > + if (!dst) { > + dst = ip6_route_output(dev_net(dev), NULL, &fl.u.ip6); > + if (dst->error) { > + dst_release(dst); > + goto tx_err; > + } > + skb_dst_set(skb, dst); > + } > break; > case htons(ETH_P_IP): > xfrm_decode_session(skb, &fl, AF_INET); > memset(IPCB(skb), 0, sizeof(*IPCB(skb))); > + if (!dst) { > + struct rtable *rt = __ip_route_output_key(dev_net(dev), > + &fl.u.ip4); > + > + if (IS_ERR(rt)) > + goto tx_err; With this change, the 'if (!dst)' in xfrmi_xmit2() is meaningless and we don't handle this error as a link failure anymore. Please make sure that the error path is not changed.
Le 09/01/2020 à 09:40, Steffen Klassert a écrit : [snip] >> @@ -352,10 +353,26 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) >> case htons(ETH_P_IPV6): >> xfrm_decode_session(skb, &fl, AF_INET6); >> memset(IP6CB(skb), 0, sizeof(*IP6CB(skb))); >> + if (!dst) { >> + dst = ip6_route_output(dev_net(dev), NULL, &fl.u.ip6); >> + if (dst->error) { >> + dst_release(dst); >> + goto tx_err; >> + } >> + skb_dst_set(skb, dst); >> + } >> break; >> case htons(ETH_P_IP): >> xfrm_decode_session(skb, &fl, AF_INET); >> memset(IPCB(skb), 0, sizeof(*IPCB(skb))); >> + if (!dst) { >> + struct rtable *rt = __ip_route_output_key(dev_net(dev), >> + &fl.u.ip4); >> + >> + if (IS_ERR(rt)) >> + goto tx_err; > > With this change, the 'if (!dst)' in xfrmi_xmit2() is meaningless Yep, I was hesitant to remove it :) > and we don't handle this error as a link failure anymore. Good point, will send a v2. Thank you, Nicolas
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c index 7ac1542feaf8..55978a1501ec 100644 --- a/net/xfrm/xfrm_interface.c +++ b/net/xfrm/xfrm_interface.c @@ -343,6 +343,7 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) { struct xfrm_if *xi = netdev_priv(dev); struct net_device_stats *stats = &xi->dev->stats; + struct dst_entry *dst = skb_dst(skb); struct flowi fl; int ret; @@ -352,10 +353,26 @@ static netdev_tx_t xfrmi_xmit(struct sk_buff *skb, struct net_device *dev) case htons(ETH_P_IPV6): xfrm_decode_session(skb, &fl, AF_INET6); memset(IP6CB(skb), 0, sizeof(*IP6CB(skb))); + if (!dst) { + dst = ip6_route_output(dev_net(dev), NULL, &fl.u.ip6); + if (dst->error) { + dst_release(dst); + goto tx_err; + } + skb_dst_set(skb, dst); + } break; case htons(ETH_P_IP): xfrm_decode_session(skb, &fl, AF_INET); memset(IPCB(skb), 0, sizeof(*IPCB(skb))); + if (!dst) { + struct rtable *rt = __ip_route_output_key(dev_net(dev), + &fl.u.ip4); + + if (IS_ERR(rt)) + goto tx_err; + skb_dst_set(skb, &rt->dst); + } break; default: goto tx_err; @@ -563,12 +580,9 @@ static void xfrmi_dev_setup(struct net_device *dev) { dev->netdev_ops = &xfrmi_netdev_ops; dev->type = ARPHRD_NONE; - dev->hard_header_len = ETH_HLEN; - dev->min_header_len = ETH_HLEN; dev->mtu = ETH_DATA_LEN; dev->min_mtu = ETH_MIN_MTU; - dev->max_mtu = ETH_DATA_LEN; - dev->addr_len = ETH_ALEN; + dev->max_mtu = IP_MAX_MTU; dev->flags = IFF_NOARP; dev->needs_free_netdev = true; dev->priv_destructor = xfrmi_dev_free;
With an ebpf program that redirects packets through a xfrm interface, packets are dropped because no dst is attached to skb. This could also be reproduced with an AF_PACKET socket, with the following python script (xfrm1 is a xfrm interface): import socket send_s = socket.socket(socket.AF_PACKET, socket.SOCK_RAW, 0) # scapy # p = IP(src='10.100.0.2', dst='10.200.0.1')/ICMP(type='echo-request') # raw(p) req = b'E\x00\x00\x1c\x00\x01\x00\x00@\x01e\xb2\nd\x00\x02\n\xc8\x00\x01\x08\x00\xf7\xff\x00\x00\x00\x00' send_s.sendto(req, ('xfrm1', 0x800, 0, 0)) It was also not possible to send an ip packet through an AF_PACKET socket because a LL header was expected. Let's remove those LL header constraints. Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> --- net/xfrm/xfrm_interface.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)