* [PATCH 01/11] xfrm: interface: fix ipv4 pmtu check to honor ip header df
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 22:00 ` patchwork-bot+netdevbpf
2021-03-31 8:18 ` [PATCH 02/11] vti: " Steffen Klassert
` (10 subsequent siblings)
11 siblings, 1 reply; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Eyal Birger <eyal.birger@gmail.com>
Frag needed should only be sent if the header enables DF.
This fix allows packets larger than MTU to pass the xfrm interface
and be fragmented after encapsulation, aligning behavior with
non-interface xfrm.
Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_interface.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 495b1f5c979b..8831f5a9e992 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -306,6 +306,8 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
icmpv6_ndo_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
} else {
+ if (!(ip_hdr(skb)->frag_off & htons(IP_DF)))
+ goto xmit;
icmp_ndo_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
htonl(mtu));
}
@@ -314,6 +316,7 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
return -EMSGSIZE;
}
+xmit:
xfrmi_scrub_packet(skb, !net_eq(xi->net, dev_net(dev)));
skb_dst_set(skb, dst);
skb->dev = tdev;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 02/11] vti: fix ipv4 pmtu check to honor ip header df
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
2021-03-31 8:18 ` [PATCH 01/11] xfrm: interface: fix ipv4 pmtu check to honor ip header df Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 03/11] vti6: " Steffen Klassert
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Eyal Birger <eyal.birger@gmail.com>
Frag needed should only be sent if the header enables DF.
This fix allows packets larger than MTU to pass the vti interface
and be fragmented after encapsulation, aligning behavior with
non-vti xfrm.
Fixes: d6af1a31cc72 ("vti: Add pmtu handling to vti_xmit.")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/ip_vti.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index abc171e79d3e..613741384490 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -218,7 +218,7 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
}
if (dst->flags & DST_XFRM_QUEUE)
- goto queued;
+ goto xmit;
if (!vti_state_check(dst->xfrm, parms->iph.daddr, parms->iph.saddr)) {
dev->stats.tx_carrier_errors++;
@@ -238,6 +238,8 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
if (skb->len > mtu) {
skb_dst_update_pmtu_no_confirm(skb, mtu);
if (skb->protocol == htons(ETH_P_IP)) {
+ if (!(ip_hdr(skb)->frag_off & htons(IP_DF)))
+ goto xmit;
icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
htonl(mtu));
} else {
@@ -251,7 +253,7 @@ static netdev_tx_t vti_xmit(struct sk_buff *skb, struct net_device *dev,
goto tx_error;
}
-queued:
+xmit:
skb_scrub_packet(skb, !net_eq(tunnel->net, dev_net(dev)));
skb_dst_set(skb, dst);
skb->dev = skb_dst(skb)->dev;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 03/11] vti6: fix ipv4 pmtu check to honor ip header df
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
2021-03-31 8:18 ` [PATCH 01/11] xfrm: interface: fix ipv4 pmtu check to honor ip header df Steffen Klassert
2021-03-31 8:18 ` [PATCH 02/11] vti: " Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 04/11] xfrm: Use actual socket sk instead of skb socket for xfrm_output_resume Steffen Klassert
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Eyal Birger <eyal.birger@gmail.com>
Frag needed should only be sent if the header enables DF.
This fix allows IPv4 packets larger than MTU to pass the vti6 interface
and be fragmented after encapsulation, aligning behavior with
non-vti6 xfrm.
Fixes: ccd740cbc6e0 ("vti6: Add pmtu handling to vti6_xmit.")
Signed-off-by: Eyal Birger <eyal.birger@gmail.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv6/ip6_vti.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index 0225fd694192..2f0be5ac021c 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -494,7 +494,7 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
}
if (dst->flags & DST_XFRM_QUEUE)
- goto queued;
+ goto xmit;
x = dst->xfrm;
if (!vti6_state_check(x, &t->parms.raddr, &t->parms.laddr))
@@ -523,6 +523,8 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
} else {
+ if (!(ip_hdr(skb)->frag_off & htons(IP_DF)))
+ goto xmit;
icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED,
htonl(mtu));
}
@@ -531,7 +533,7 @@ vti6_xmit(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
goto tx_err_dst_release;
}
-queued:
+xmit:
skb_scrub_packet(skb, !net_eq(t->net, dev_net(dev)));
skb_dst_set(skb, dst);
skb->dev = skb_dst(skb)->dev;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 04/11] xfrm: Use actual socket sk instead of skb socket for xfrm_output_resume
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (2 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 03/11] vti6: " Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 05/11] net: xfrm: Localize sequence counter per network namespace Steffen Klassert
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Evan Nimmo <evan.nimmo@alliedtelesis.co.nz>
A situation can occur where the interface bound to the sk is different
to the interface bound to the sk attached to the skb. The interface
bound to the sk is the correct one however this information is lost inside
xfrm_output2 and instead the sk on the skb is used in xfrm_output_resume
instead. This assumes that the sk bound interface and the bound interface
attached to the sk within the skb are the same which can lead to lookup
failures inside ip_route_me_harder resulting in the packet being dropped.
We have an l2tp v3 tunnel with ipsec protection. The tunnel is in the
global VRF however we have an encapsulated dot1q tunnel interface that
is within a different VRF. We also have a mangle rule that marks the
packets causing them to be processed inside ip_route_me_harder.
Prior to commit 31c70d5956fc ("l2tp: keep original skb ownership") this
worked fine as the sk attached to the skb was changed from the dot1q
encapsulated interface to the sk for the tunnel which meant the interface
bound to the sk and the interface bound to the skb were identical.
Commit 46d6c5ae953c ("netfilter: use actual socket sk rather than skb sk
when routing harder") fixed some of these issues however a similar
problem existed in the xfrm code.
Fixes: 31c70d5956fc ("l2tp: keep original skb ownership")
Signed-off-by: Evan Nimmo <evan.nimmo@alliedtelesis.co.nz>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 2 +-
net/ipv4/ah4.c | 2 +-
net/ipv4/esp4.c | 2 +-
net/ipv6/ah6.c | 2 +-
net/ipv6/esp6.c | 2 +-
net/xfrm/xfrm_output.c | 10 +++++-----
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index b2a06f10b62c..bfbc7810df94 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1557,7 +1557,7 @@ int xfrm_trans_queue_net(struct net *net, struct sk_buff *skb,
int xfrm_trans_queue(struct sk_buff *skb,
int (*finish)(struct net *, struct sock *,
struct sk_buff *));
-int xfrm_output_resume(struct sk_buff *skb, int err);
+int xfrm_output_resume(struct sock *sk, struct sk_buff *skb, int err);
int xfrm_output(struct sock *sk, struct sk_buff *skb);
#if IS_ENABLED(CONFIG_NET_PKTGEN)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index d99e1be94019..36ed85bf2ad5 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -141,7 +141,7 @@ static void ah_output_done(struct crypto_async_request *base, int err)
}
kfree(AH_SKB_CB(skb)->tmp);
- xfrm_output_resume(skb, err);
+ xfrm_output_resume(skb->sk, skb, err);
}
static int ah_output(struct xfrm_state *x, struct sk_buff *skb)
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index a3271ec3e162..4b834bbf95e0 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -279,7 +279,7 @@ static void esp_output_done(struct crypto_async_request *base, int err)
x->encap && x->encap->encap_type == TCP_ENCAP_ESPINTCP)
esp_output_tail_tcp(x, skb);
else
- xfrm_output_resume(skb, err);
+ xfrm_output_resume(skb->sk, skb, err);
}
}
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 440080da805b..080ee7f44c64 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -316,7 +316,7 @@ static void ah6_output_done(struct crypto_async_request *base, int err)
}
kfree(AH_SKB_CB(skb)->tmp);
- xfrm_output_resume(skb, err);
+ xfrm_output_resume(skb->sk, skb, err);
}
static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 153ad103ba74..727d791ed5e6 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -314,7 +314,7 @@ static void esp_output_done(struct crypto_async_request *base, int err)
x->encap && x->encap->encap_type == TCP_ENCAP_ESPINTCP)
esp_output_tail_tcp(x, skb);
else
- xfrm_output_resume(skb, err);
+ xfrm_output_resume(skb->sk, skb, err);
}
}
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index a7ab19353313..b81ca117dac7 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -503,22 +503,22 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
return err;
}
-int xfrm_output_resume(struct sk_buff *skb, int err)
+int xfrm_output_resume(struct sock *sk, struct sk_buff *skb, int err)
{
struct net *net = xs_net(skb_dst(skb)->xfrm);
while (likely((err = xfrm_output_one(skb, err)) == 0)) {
nf_reset_ct(skb);
- err = skb_dst(skb)->ops->local_out(net, skb->sk, skb);
+ err = skb_dst(skb)->ops->local_out(net, sk, skb);
if (unlikely(err != 1))
goto out;
if (!skb_dst(skb)->xfrm)
- return dst_output(net, skb->sk, skb);
+ return dst_output(net, sk, skb);
err = nf_hook(skb_dst(skb)->ops->family,
- NF_INET_POST_ROUTING, net, skb->sk, skb,
+ NF_INET_POST_ROUTING, net, sk, skb,
NULL, skb_dst(skb)->dev, xfrm_output2);
if (unlikely(err != 1))
goto out;
@@ -534,7 +534,7 @@ EXPORT_SYMBOL_GPL(xfrm_output_resume);
static int xfrm_output2(struct net *net, struct sock *sk, struct sk_buff *skb)
{
- return xfrm_output_resume(skb, 1);
+ return xfrm_output_resume(sk, skb, 1);
}
static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb)
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 05/11] net: xfrm: Localize sequence counter per network namespace
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (3 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 04/11] xfrm: Use actual socket sk instead of skb socket for xfrm_output_resume Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 06/11] net: xfrm: Use sequence counter with associated spinlock Steffen Klassert
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
A sequence counter write section must be serialized or its internal
state can get corrupted. The "xfrm_state_hash_generation" seqcount is
global, but its write serialization lock (net->xfrm.xfrm_state_lock) is
instantiated per network namespace. The write protection is thus
insufficient.
To provide full protection, localize the sequence counter per network
namespace instead. This should be safe as both the seqcount read and
write sections access data exclusively within the network namespace. It
also lays the foundation for transforming "xfrm_state_hash_generation"
data type from seqcount_t to seqcount_LOCKNAME_t in further commits.
Fixes: b65e3d7be06f ("xfrm: state: add sequence count to detect hash resizes")
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/netns/xfrm.h | 4 +++-
net/xfrm/xfrm_state.c | 10 +++++-----
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index 59f45b1e9dac..b59d73d529ba 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -72,7 +72,9 @@ struct netns_xfrm {
#if IS_ENABLED(CONFIG_IPV6)
struct dst_ops xfrm6_dst_ops;
#endif
- spinlock_t xfrm_state_lock;
+ spinlock_t xfrm_state_lock;
+ seqcount_t xfrm_state_hash_generation;
+
spinlock_t xfrm_policy_lock;
struct mutex xfrm_cfg_mutex;
};
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index d01ca1a18418..ffd315cff984 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -44,7 +44,6 @@ static void xfrm_state_gc_task(struct work_struct *work);
*/
static unsigned int xfrm_state_hashmax __read_mostly = 1 * 1024 * 1024;
-static __read_mostly seqcount_t xfrm_state_hash_generation = SEQCNT_ZERO(xfrm_state_hash_generation);
static struct kmem_cache *xfrm_state_cache __ro_after_init;
static DECLARE_WORK(xfrm_state_gc_work, xfrm_state_gc_task);
@@ -140,7 +139,7 @@ static void xfrm_hash_resize(struct work_struct *work)
}
spin_lock_bh(&net->xfrm.xfrm_state_lock);
- write_seqcount_begin(&xfrm_state_hash_generation);
+ write_seqcount_begin(&net->xfrm.xfrm_state_hash_generation);
nhashmask = (nsize / sizeof(struct hlist_head)) - 1U;
odst = xfrm_state_deref_prot(net->xfrm.state_bydst, net);
@@ -156,7 +155,7 @@ static void xfrm_hash_resize(struct work_struct *work)
rcu_assign_pointer(net->xfrm.state_byspi, nspi);
net->xfrm.state_hmask = nhashmask;
- write_seqcount_end(&xfrm_state_hash_generation);
+ write_seqcount_end(&net->xfrm.xfrm_state_hash_generation);
spin_unlock_bh(&net->xfrm.xfrm_state_lock);
osize = (ohashmask + 1) * sizeof(struct hlist_head);
@@ -1063,7 +1062,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
to_put = NULL;
- sequence = read_seqcount_begin(&xfrm_state_hash_generation);
+ sequence = read_seqcount_begin(&net->xfrm.xfrm_state_hash_generation);
rcu_read_lock();
h = xfrm_dst_hash(net, daddr, saddr, tmpl->reqid, encap_family);
@@ -1176,7 +1175,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
if (to_put)
xfrm_state_put(to_put);
- if (read_seqcount_retry(&xfrm_state_hash_generation, sequence)) {
+ if (read_seqcount_retry(&net->xfrm.xfrm_state_hash_generation, sequence)) {
*err = -EAGAIN;
if (x) {
xfrm_state_put(x);
@@ -2666,6 +2665,7 @@ int __net_init xfrm_state_init(struct net *net)
net->xfrm.state_num = 0;
INIT_WORK(&net->xfrm.state_hash_work, xfrm_hash_resize);
spin_lock_init(&net->xfrm.xfrm_state_lock);
+ seqcount_init(&net->xfrm.xfrm_state_hash_generation);
return 0;
out_byspi:
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 06/11] net: xfrm: Use sequence counter with associated spinlock
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (4 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 05/11] net: xfrm: Localize sequence counter per network namespace Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 07/11] esp: delete NETIF_F_SCTP_CRC bit from features for esp offload Steffen Klassert
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: "Ahmed S. Darwish" <a.darwish@linutronix.de>
A sequence counter write section must be serialized or its internal
state can get corrupted. A plain seqcount_t does not contain the
information of which lock must be held to guaranteee write side
serialization.
For xfrm_state_hash_generation, use seqcount_spinlock_t instead of plain
seqcount_t. This allows to associate the spinlock used for write
serialization with the sequence counter. It thus enables lockdep to
verify that the write serialization lock is indeed held before entering
the sequence counter write section.
If lockdep is disabled, this lock association is compiled out and has
neither storage size nor runtime overhead.
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/netns/xfrm.h | 2 +-
net/xfrm/xfrm_state.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/net/netns/xfrm.h b/include/net/netns/xfrm.h
index b59d73d529ba..e816b6a3ef2b 100644
--- a/include/net/netns/xfrm.h
+++ b/include/net/netns/xfrm.h
@@ -73,7 +73,7 @@ struct netns_xfrm {
struct dst_ops xfrm6_dst_ops;
#endif
spinlock_t xfrm_state_lock;
- seqcount_t xfrm_state_hash_generation;
+ seqcount_spinlock_t xfrm_state_hash_generation;
spinlock_t xfrm_policy_lock;
struct mutex xfrm_cfg_mutex;
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index ffd315cff984..4496f7efa220 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2665,7 +2665,8 @@ int __net_init xfrm_state_init(struct net *net)
net->xfrm.state_num = 0;
INIT_WORK(&net->xfrm.state_hash_work, xfrm_hash_resize);
spin_lock_init(&net->xfrm.xfrm_state_lock);
- seqcount_init(&net->xfrm.xfrm_state_hash_generation);
+ seqcount_spinlock_init(&net->xfrm.xfrm_state_hash_generation,
+ &net->xfrm.xfrm_state_lock);
return 0;
out_byspi:
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 07/11] esp: delete NETIF_F_SCTP_CRC bit from features for esp offload
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (5 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 06/11] net: xfrm: Use sequence counter with associated spinlock Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 08/11] xfrm: BEET mode doesn't support fragments for inner packets Steffen Klassert
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Xin Long <lucien.xin@gmail.com>
Now in esp4/6_gso_segment(), before calling inner proto .gso_segment,
NETIF_F_CSUM_MASK bits are deleted, as HW won't be able to do the
csum for inner proto due to the packet encrypted already.
So the UDP/TCP packet has to do the checksum on its own .gso_segment.
But SCTP is using CRC checksum, and for that NETIF_F_SCTP_CRC should
be deleted to make SCTP do the csum in own .gso_segment as well.
In Xiumei's testing with SCTP over IPsec/veth, the packets are kept
dropping due to the wrong CRC checksum.
Reported-by: Xiumei Mu <xmu@redhat.com>
Fixes: 7862b4058b9f ("esp: Add gso handlers for esp4 and esp6")
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/esp4_offload.c | 6 ++++--
net/ipv6/esp6_offload.c | 6 ++++--
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index 601f5fbfc63f..ed3de486ea34 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -217,10 +217,12 @@ static struct sk_buff *esp4_gso_segment(struct sk_buff *skb,
if ((!(skb->dev->gso_partial_features & NETIF_F_HW_ESP) &&
!(features & NETIF_F_HW_ESP)) || x->xso.dev != skb->dev)
- esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK);
+ esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK |
+ NETIF_F_SCTP_CRC);
else if (!(features & NETIF_F_HW_ESP_TX_CSUM) &&
!(skb->dev->gso_partial_features & NETIF_F_HW_ESP_TX_CSUM))
- esp_features = features & ~NETIF_F_CSUM_MASK;
+ esp_features = features & ~(NETIF_F_CSUM_MASK |
+ NETIF_F_SCTP_CRC);
xo->flags |= XFRM_GSO_SEGMENT;
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 1ca516fb30e1..f35203ab39f5 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -254,9 +254,11 @@ static struct sk_buff *esp6_gso_segment(struct sk_buff *skb,
skb->encap_hdr_csum = 1;
if (!(features & NETIF_F_HW_ESP) || x->xso.dev != skb->dev)
- esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK);
+ esp_features = features & ~(NETIF_F_SG | NETIF_F_CSUM_MASK |
+ NETIF_F_SCTP_CRC);
else if (!(features & NETIF_F_HW_ESP_TX_CSUM))
- esp_features = features & ~NETIF_F_CSUM_MASK;
+ esp_features = features & ~(NETIF_F_CSUM_MASK |
+ NETIF_F_SCTP_CRC);
xo->flags |= XFRM_GSO_SEGMENT;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 08/11] xfrm: BEET mode doesn't support fragments for inner packets
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (6 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 07/11] esp: delete NETIF_F_SCTP_CRC bit from features for esp offload Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 09/11] xfrm: Fix NULL pointer dereference on policy lookup Steffen Klassert
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Xin Long <lucien.xin@gmail.com>
BEET mode replaces the IP(6) Headers with new IP(6) Headers when sending
packets. However, when it's a fragment before the replacement, currently
kernel keeps the fragment flag and replace the address field then encaps
it with ESP. It would cause in RX side the fragments to get reassembled
before decapping with ESP, which is incorrect.
In Xiumei's testing, these fragments went over an xfrm interface and got
encapped with ESP in the device driver, and the traffic was broken.
I don't have a good way to fix it, but only to warn this out in dmesg.
Reported-by: Xiumei Mu <xmu@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_output.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index b81ca117dac7..e4cb0ff4dcf4 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -660,6 +660,12 @@ static int xfrm4_extract_output(struct xfrm_state *x, struct sk_buff *skb)
{
int err;
+ if (x->outer_mode.encap == XFRM_MODE_BEET &&
+ ip_is_fragment(ip_hdr(skb))) {
+ net_warn_ratelimited("BEET mode doesn't support inner IPv4 fragments\n");
+ return -EAFNOSUPPORT;
+ }
+
err = xfrm4_tunnel_check_size(skb);
if (err)
return err;
@@ -705,8 +711,15 @@ static int xfrm6_tunnel_check_size(struct sk_buff *skb)
static int xfrm6_extract_output(struct xfrm_state *x, struct sk_buff *skb)
{
#if IS_ENABLED(CONFIG_IPV6)
+ unsigned int ptr = 0;
int err;
+ if (x->outer_mode.encap == XFRM_MODE_BEET &&
+ ipv6_find_hdr(skb, &ptr, NEXTHDR_FRAGMENT, NULL, NULL) >= 0) {
+ net_warn_ratelimited("BEET mode doesn't support inner IPv6 fragments\n");
+ return -EAFNOSUPPORT;
+ }
+
err = xfrm6_tunnel_check_size(skb);
if (err)
return err;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 09/11] xfrm: Fix NULL pointer dereference on policy lookup
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (7 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 08/11] xfrm: BEET mode doesn't support fragments for inner packets Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 10/11] xfrm: Provide private skb extensions for segmented and hw offloaded ESP packets Steffen Klassert
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
When xfrm interfaces are used in combination with namespaces
and ESP offload, we get a dst_entry NULL pointer dereference.
This is because we don't have a dst_entry attached in the ESP
offloading case and we need to do a policy lookup before the
namespace transition.
Fix this by expicit checking of skb_dst(skb) before accessing it.
Fixes: f203b76d78092 ("xfrm: Add virtual xfrm interfaces")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index bfbc7810df94..c58a6d4eb610 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1097,7 +1097,7 @@ static inline int __xfrm_policy_check2(struct sock *sk, int dir,
return __xfrm_policy_check(sk, ndir, skb, family);
return (!net->xfrm.policy_count[dir] && !secpath_exists(skb)) ||
- (skb_dst(skb)->flags & DST_NOPOLICY) ||
+ (skb_dst(skb) && (skb_dst(skb)->flags & DST_NOPOLICY)) ||
__xfrm_policy_check(sk, ndir, skb, family);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 10/11] xfrm: Provide private skb extensions for segmented and hw offloaded ESP packets
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (8 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 09/11] xfrm: Fix NULL pointer dereference on policy lookup Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 8:18 ` [PATCH 11/11] xfrm/compat: Cleanup WARN()s that can be user-triggered Steffen Klassert
2021-03-31 22:00 ` pull request (net): ipsec 2021-03-31 patchwork-bot+netdevbpf
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
Commit 94579ac3f6d0 ("xfrm: Fix double ESP trailer insertion in IPsec
crypto offload.") added a XFRM_XMIT flag to avoid duplicate ESP trailer
insertion on HW offload. This flag is set on the secpath that is shared
amongst segments. This lead to a situation where some segments are
not transformed correctly when segmentation happens at layer 3.
Fix this by using private skb extensions for segmented and hw offloaded
ESP packets.
Fixes: 94579ac3f6d0 ("xfrm: Fix double ESP trailer insertion in IPsec crypto offload.")
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/esp4_offload.c | 11 ++++++++++-
net/ipv6/esp6_offload.c | 11 ++++++++++-
net/xfrm/xfrm_device.c | 2 --
3 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index ed3de486ea34..33687cf58286 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -314,8 +314,17 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb, netdev_features_
ip_hdr(skb)->tot_len = htons(skb->len);
ip_send_check(ip_hdr(skb));
- if (hw_offload)
+ if (hw_offload) {
+ if (!skb_ext_add(skb, SKB_EXT_SEC_PATH))
+ return -ENOMEM;
+
+ xo = xfrm_offload(skb);
+ if (!xo)
+ return -EINVAL;
+
+ xo->flags |= XFRM_XMIT;
return 0;
+ }
err = esp_output_tail(x, skb, &esp);
if (err)
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index f35203ab39f5..4af56affaafd 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -348,8 +348,17 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb, netdev_features
ipv6_hdr(skb)->payload_len = htons(len);
- if (hw_offload)
+ if (hw_offload) {
+ if (!skb_ext_add(skb, SKB_EXT_SEC_PATH))
+ return -ENOMEM;
+
+ xo = xfrm_offload(skb);
+ if (!xo)
+ return -EINVAL;
+
+ xo->flags |= XFRM_XMIT;
return 0;
+ }
err = esp6_output_tail(x, skb, &esp);
if (err)
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index edf11893dbe8..6d6917b68856 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -134,8 +134,6 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
return skb;
}
- xo->flags |= XFRM_XMIT;
-
if (skb_is_gso(skb) && unlikely(x->xso.dev != dev)) {
struct sk_buff *segs;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 11/11] xfrm/compat: Cleanup WARN()s that can be user-triggered
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (9 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 10/11] xfrm: Provide private skb extensions for segmented and hw offloaded ESP packets Steffen Klassert
@ 2021-03-31 8:18 ` Steffen Klassert
2021-03-31 22:00 ` pull request (net): ipsec 2021-03-31 patchwork-bot+netdevbpf
11 siblings, 0 replies; 14+ messages in thread
From: Steffen Klassert @ 2021-03-31 8:18 UTC (permalink / raw)
To: David Miller, Jakub Kicinski; +Cc: Herbert Xu, Steffen Klassert, netdev
From: Dmitry Safonov <dima@arista.com>
Replace WARN_ONCE() that can be triggered from userspace with
pr_warn_once(). Those still give user a hint what's the issue.
I've left WARN()s that are not possible to trigger with current
code-base and that would mean that the code has issues:
- relying on current compat_msg_min[type] <= xfrm_msg_min[type]
- expected 4-byte padding size difference between
compat_msg_min[type] and xfrm_msg_min[type]
- compat_policy[type].len <= xfrma_policy[type].len
(for every type)
Reported-by: syzbot+834ffd1afc7212eb8147@syzkaller.appspotmail.com
Fixes: 5f3eea6b7e8f ("xfrm/compat: Attach xfrm dumps to 64=>32 bit translator")
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: netdev@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Dmitry Safonov <dima@arista.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_compat.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/net/xfrm/xfrm_compat.c b/net/xfrm/xfrm_compat.c
index d8e8a11ca845..a20aec9d7393 100644
--- a/net/xfrm/xfrm_compat.c
+++ b/net/xfrm/xfrm_compat.c
@@ -216,7 +216,7 @@ static struct nlmsghdr *xfrm_nlmsg_put_compat(struct sk_buff *skb,
case XFRM_MSG_GETSADINFO:
case XFRM_MSG_GETSPDINFO:
default:
- WARN_ONCE(1, "unsupported nlmsg_type %d", nlh_src->nlmsg_type);
+ pr_warn_once("unsupported nlmsg_type %d\n", nlh_src->nlmsg_type);
return ERR_PTR(-EOPNOTSUPP);
}
@@ -277,7 +277,7 @@ static int xfrm_xlate64_attr(struct sk_buff *dst, const struct nlattr *src)
return xfrm_nla_cpy(dst, src, nla_len(src));
default:
BUILD_BUG_ON(XFRMA_MAX != XFRMA_IF_ID);
- WARN_ONCE(1, "unsupported nla_type %d", src->nla_type);
+ pr_warn_once("unsupported nla_type %d\n", src->nla_type);
return -EOPNOTSUPP;
}
}
@@ -315,8 +315,10 @@ static int xfrm_alloc_compat(struct sk_buff *skb, const struct nlmsghdr *nlh_src
struct sk_buff *new = NULL;
int err;
- if (WARN_ON_ONCE(type >= ARRAY_SIZE(xfrm_msg_min)))
+ if (type >= ARRAY_SIZE(xfrm_msg_min)) {
+ pr_warn_once("unsupported nlmsg_type %d\n", nlh_src->nlmsg_type);
return -EOPNOTSUPP;
+ }
if (skb_shinfo(skb)->frag_list == NULL) {
new = alloc_skb(skb->len + skb_tailroom(skb), GFP_ATOMIC);
@@ -378,6 +380,10 @@ static int xfrm_attr_cpy32(void *dst, size_t *pos, const struct nlattr *src,
struct nlmsghdr *nlmsg = dst;
struct nlattr *nla;
+ /* xfrm_user_rcv_msg_compat() relies on fact that 32-bit messages
+ * have the same len or shorted than 64-bit ones.
+ * 32-bit translation that is bigger than 64-bit original is unexpected.
+ */
if (WARN_ON_ONCE(copy_len > payload))
copy_len = payload;
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: pull request (net): ipsec 2021-03-31
2021-03-31 8:18 pull request (net): ipsec 2021-03-31 Steffen Klassert
` (10 preceding siblings ...)
2021-03-31 8:18 ` [PATCH 11/11] xfrm/compat: Cleanup WARN()s that can be user-triggered Steffen Klassert
@ 2021-03-31 22:00 ` patchwork-bot+netdevbpf
11 siblings, 0 replies; 14+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-03-31 22:00 UTC (permalink / raw)
To: Steffen Klassert; +Cc: davem, kuba, herbert, netdev
Hello:
This pull request was applied to netdev/net.git (refs/heads/master):
On Wed, 31 Mar 2021 10:18:36 +0200 you wrote:
> 1) Fix ipv4 pmtu checks for xfrm anf vti interfaces.
> From Eyal Birger.
>
> 2) There are situations where the socket passed to
> xfrm_output_resume() is not the same as the one
> attached to the skb. Use the socket passed to
> xfrm_output_resume() to avoid lookup failures
> when xfrm is used with VRFs.
> From Evan Nimmo.
>
> [...]
Here is the summary with links:
- pull request (net): ipsec 2021-03-31
https://git.kernel.org/netdev/net/c/c9170f132178
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 14+ messages in thread