* [PATCH net V2] net: ipv6: Validate GSO SKB before finish IPv6 processing
@ 2021-01-05 13:43 Aya Levin
2021-01-06 23:52 ` Jakub Kicinski
0 siblings, 1 reply; 2+ messages in thread
From: Aya Levin @ 2021-01-05 13:43 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski
Cc: Daniel Axtens, netdev, Moshe Shemesh, Tariq Toukan,
Dan Carpenter, Aya Levin
There are cases where GSO segment's length exceeds the egress MTU.
If so:
- Consume the SKB and its segments.
- Issue an ICMP packet with 'Packet Too Big' message containing the
MTU, allowing the source host to reduce its Path MTU appropriately.
Note: These cases are handled in the same manner in IPv4 output finish.
This patch aligns the behavior of IPv6 and the one of IPv4.
Fixes: 9e50849054a4 ("netfilter: ipv6: move POSTROUTING invocation before fragmentation")
Signed-off-by: Aya Levin <ayal@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
---
net/ipv6/ip6_output.c | 41 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
Hi,
Please queue to -stable >= v2.6.35.
Thanks,
Aya
v2:
Addressed error: uninitialized symbol 'ret', reported by
kernel test robot <lkp@intel.com> / Dan Carpenter <dan.carpenter@oracle.com>.
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 749ad72386b2..077d43af8226 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -125,8 +125,43 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
return -EINVAL;
}
+static int
+ip6_finish_output_gso_slowpath_drop(struct net *net, struct sock *sk,
+ struct sk_buff *skb, unsigned int mtu)
+{
+ struct sk_buff *segs, *nskb;
+ netdev_features_t features;
+ int ret = 0;
+
+ /* Please see corresponding comment in ip_finish_output_gso
+ * describing the cases where GSO segment length exceeds the
+ * egress MTU.
+ */
+ features = netif_skb_features(skb);
+ segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+ if (IS_ERR_OR_NULL(segs)) {
+ kfree_skb(skb);
+ return -ENOMEM;
+ }
+
+ consume_skb(skb);
+
+ skb_list_walk_safe(segs, segs, nskb) {
+ int err;
+
+ skb_mark_not_on_list(segs);
+ err = ip6_fragment(net, sk, segs, ip6_finish_output2);
+ if (err && ret == 0)
+ ret = err;
+ }
+
+ return ret;
+}
+
static int __ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff *skb)
{
+ unsigned int mtu;
+
#if defined(CONFIG_NETFILTER) && defined(CONFIG_XFRM)
/* Policy lookup after SNAT yielded a new policy */
if (skb_dst(skb)->xfrm) {
@@ -135,7 +170,11 @@ static int __ip6_finish_output(struct net *net, struct sock *sk, struct sk_buff
}
#endif
- if ((skb->len > ip6_skb_dst_mtu(skb) && !skb_is_gso(skb)) ||
+ mtu = ip6_skb_dst_mtu(skb);
+ if (skb_is_gso(skb) && !skb_gso_validate_network_len(skb, mtu))
+ return ip6_finish_output_gso_slowpath_drop(net, sk, skb, mtu);
+
+ if ((skb->len > mtu && !skb_is_gso(skb)) ||
dst_allfrag(skb_dst(skb)) ||
(IP6CB(skb)->frag_max_size && skb->len > IP6CB(skb)->frag_max_size))
return ip6_fragment(net, sk, skb, ip6_finish_output2);
--
2.14.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net V2] net: ipv6: Validate GSO SKB before finish IPv6 processing
2021-01-05 13:43 [PATCH net V2] net: ipv6: Validate GSO SKB before finish IPv6 processing Aya Levin
@ 2021-01-06 23:52 ` Jakub Kicinski
0 siblings, 0 replies; 2+ messages in thread
From: Jakub Kicinski @ 2021-01-06 23:52 UTC (permalink / raw)
To: Aya Levin
Cc: David S. Miller, Daniel Axtens, netdev, Moshe Shemesh,
Tariq Toukan, Dan Carpenter
On Tue, 5 Jan 2021 15:43:21 +0200 Aya Levin wrote:
> There are cases where GSO segment's length exceeds the egress MTU.
Please name same for posterity.
Please widen the CC list.
> If so:
> - Consume the SKB and its segments.
> - Issue an ICMP packet with 'Packet Too Big' message containing the
> MTU, allowing the source host to reduce its Path MTU appropriately.
>
> Note: These cases are handled in the same manner in IPv4 output finish.
> This patch aligns the behavior of IPv6 and the one of IPv4.
>
> Fixes: 9e50849054a4 ("netfilter: ipv6: move POSTROUTING invocation before fragmentation")
Not sure if this commit actually adds the problem or just moves the
problematic snippet around. But probably doesn't matter that much for
10+ years old code.
> Signed-off-by: Aya Levin <ayal@nvidia.com>
> Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
The code itself looks reasonable AFAICT.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-01-06 23:52 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 13:43 [PATCH net V2] net: ipv6: Validate GSO SKB before finish IPv6 processing Aya Levin
2021-01-06 23:52 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.