Message ID | 20220416122434.33061-1-sven@narfation.org |
---|---|
State | Accepted, archived |
Delegated to: | Simon Wunderlich |
Headers |
Return-Path: <b.a.t.m.a.n-bounces@lists.open-mesh.org> X-Original-To: patchwork@open-mesh.org Delivered-To: patchwork@open-mesh.org Received: from diktynna.open-mesh.org (localhost [IPv6:::1]) by diktynna.open-mesh.org (Postfix) with ESMTP id A408F82E4A; Sat, 16 Apr 2022 14:25:09 +0200 (CEST) Received: from dvalin.narfation.org (dvalin.narfation.org [IPv6:2a00:17d8:100::8b1]) by diktynna.open-mesh.org (Postfix) with ESMTPS id 1A5358072F for <b.a.t.m.a.n@lists.open-mesh.org>; Sat, 16 Apr 2022 14:25:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=narfation.org; s=20121; t=1650111906; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=5gRVsR+k88icLnAey0T70AAH6oNT5lNOUkoxchzsRPM=; b=i0RK3PU/DoVn4u0RdPUi7bseSOcd+0YYoxH0rqT2Zu296Dfi8fyxkovp9uxEdMik15dVdM UzqxRdZpG8Q1C9+Fi4KXpvdvQ6dYaxzn9lHzSY2C9b60zLYmjqfWoEsYTXEE2vKcTiUh+X HfIW5qdgQtJ9O3bRUAwHVZ53VMfUEJg= From: Sven Eckelmann <sven@narfation.org> To: b.a.t.m.a.n@lists.open-mesh.org Cc: Sven Eckelmann <sven@narfation.org>, Felix Kaechele <felix@kaechele.ca> Subject: [PATCH] batman-adv: Don't skb_split skbuffs with frag_list Date: Sat, 16 Apr 2022 14:24:34 +0200 Message-Id: <20220416122434.33061-1-sven@narfation.org> X-Mailer: git-send-email 2.30.2 MIME-Version: 1.0 ARC-Seal: i=1; s=20121; d=open-mesh.org; t=1650111907; a=rsa-sha256; cv=none; b=tQJgHBI3wdlZ7sGXb9IPyS+tkZjT2+paYv5a1AcAojj2jaxqInnnRGwh0h2wQBWJUaMqUb WFRuDs/+T9dN86IPAhe1Oi21H1V3A5i87u7rloHneoEnbYSqzoypLz3+TVE/eTTGI1ln+K uQK2LQaJEi0l+KaFaNuQzKxQT1y7BiE= ARC-Authentication-Results: i=1; diktynna.open-mesh.org; dkim=pass header.d=narfation.org header.s=20121 header.b="i0RK3PU/"; spf=pass (diktynna.open-mesh.org: domain of sven@narfation.org designates 2a00:17d8:100::8b1 as permitted sender) smtp.mailfrom=sven@narfation.org; dmarc=pass (policy=none) header.from=narfation.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=open-mesh.org; s=20121; t=1650111907; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding:dkim-signature; bh=5gRVsR+k88icLnAey0T70AAH6oNT5lNOUkoxchzsRPM=; b=dWCQpUmbJRNQmGO6kGLrF37i7+uPX3e2m9TZJt6Kt5TKaVVAD8rdgsRFCO36AFex6n70Sa zNg3orcms8e2MzhVbZKgfr4m3KCsqBau3R9XB+/fQ56wSfp+uYGO20ftOZqK4koTYLWDNn bBRVQ5hjpAnogoWrlm1w5kfrxKExXpY= Content-Transfer-Encoding: quoted-printable Message-ID-Hash: VBWJGIW5MY6QZBWLESUVN2DJZBI4XKWD X-Message-ID-Hash: VBWJGIW5MY6QZBWLESUVN2DJZBI4XKWD X-MailFrom: sven@narfation.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-b.a.t.m.a.n.lists.open-mesh.org-0; header-match-b.a.t.m.a.n.lists.open-mesh.org-1; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.1 Precedence: list Reply-To: The list for a Better Approach To Mobile Ad-hoc Networking <b.a.t.m.a.n@lists.open-mesh.org> List-Id: The list for a Better Approach To Mobile Ad-hoc Networking <b.a.t.m.a.n.lists.open-mesh.org> Archived-At: <https://lists.open-mesh.org/mailman3/hyperkitty/list/b.a.t.m.a.n@lists.open-mesh.org/message/VBWJGIW5MY6QZBWLESUVN2DJZBI4XKWD/> List-Archive: <https://lists.open-mesh.org/mailman3/hyperkitty/list/b.a.t.m.a.n@lists.open-mesh.org/> List-Help: <mailto:b.a.t.m.a.n-request@lists.open-mesh.org?subject=help> List-Post: <mailto:b.a.t.m.a.n@lists.open-mesh.org> List-Subscribe: <mailto:b.a.t.m.a.n-join@lists.open-mesh.org> List-Unsubscribe: <mailto:b.a.t.m.a.n-leave@lists.open-mesh.org> |
Series |
batman-adv: Don't skb_split skbuffs with frag_list
|
|
Commit Message
Sven Eckelmann
April 16, 2022, 12:24 p.m. UTC
The receiving interface might have used GRO to receive more fragments than
MAX_SKB_FRAGS fragments. In this case, these will not be stored in
skb_shinfo(skb)->frags but merged into the frag list.
batman-adv relies on the function skb_split to split packets up into
multiple smaller packets which are not larger than the MTU on the outgoing
interface. But this function cannot handle frag_list entries and is only
operating on skb_shinfo(skb)->frags. If it is then still trying to split
such an skb and xmit'ing it on an interface without support for
NETIF_F_FRAGLIST then validate_xmit_skb() will try to linearize it. But
this fails due to inconsistent information and __pskb_pull_tail will
trigger a BUG_ON after skb_copy_bits() returns an error.
In case of entries in frag_list, just linearize the skb before operating on
it with skb_split().
Reported-by: Felix Kaechele <felix@kaechele.ca>
Fixes: 9de347143505 ("batman-adv: layer2 unicast packet fragmentation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
net/batman-adv/fragmentation.c | 11 +++++++++++
1 file changed, 11 insertions(+)
Comments
The sender domain has a DMARC Reject/Quarantine policy which disallows sending mailing list messages using the original "From" header. To mitigate this problem, the original message has been wrapped automatically by the mailing list software. Hi there, initial testing shows that this patch seems to fix the issue. We are currently at 30 minutes of uptime on our fairly busy mesh which is already 15-30 times better than before. Thanks for the super quick turnaround on this one, especially on easter weekend. I will report back after some more uptime, but I have a feeling that if it is working right now it will probably continue to function just fine. Thanks again! Regards, Felix
On Sat, Apr 16, 2022 at 02:24:34PM +0200, Sven Eckelmann wrote: > The receiving interface might have used GRO to receive more fragments than > MAX_SKB_FRAGS fragments. In this case, these will not be stored in > skb_shinfo(skb)->frags but merged into the frag list. > > batman-adv relies on the function skb_split to split packets up into > multiple smaller packets which are not larger than the MTU on the outgoing > interface. But this function cannot handle frag_list entries and is only > operating on skb_shinfo(skb)->frags. If it is then still trying to split > such an skb and xmit'ing it on an interface without support for > NETIF_F_FRAGLIST then validate_xmit_skb() will try to linearize it. But > this fails due to inconsistent information and __pskb_pull_tail will > trigger a BUG_ON after skb_copy_bits() returns an error. > > In case of entries in frag_list, just linearize the skb before operating on > it with skb_split(). Hi Sven This is not an area of the kernel i'm very familiar with. But i'm wondering, is this a BATMAN specific problem, or a generic problem? Should the fix be in BATMAN, or the core? Andrew
On Saturday, 16 April 2022 16:21:19 CEST Andrew Lunn wrote: > This is not an area of the kernel i'm very familiar with. But i'm > wondering, is this a BATMAN specific problem, or a generic problem? > Should the fix be in BATMAN, or the core? I understand what you mean. But let me cite two places which required to operate on parts of the frag lists: /* If we need update frag list, we are in troubles. * Certainly, it is possible to add an offset to skb data, * but taking into account that pulling is expected to * be very rare operation, it is worth to fight against * further bloating skb head and crucify ourselves here instead. * Pure masohism, indeed. 8)8) */ /* Misery. We are in troubles, going to mincer fragments... */ And since I cannot reproduce this here at the moment, I've decided not to start with that. Kind regards, Sven
On Sat, Apr 16, 2022 at 07:17:28PM +0200, Sven Eckelmann wrote: > On Saturday, 16 April 2022 16:21:19 CEST Andrew Lunn wrote: > > This is not an area of the kernel i'm very familiar with. But i'm > > wondering, is this a BATMAN specific problem, or a generic problem? > > Should the fix be in BATMAN, or the core? > > I understand what you mean. But let me cite two places which required to > operate on parts of the frag lists: > > /* If we need update frag list, we are in troubles. > * Certainly, it is possible to add an offset to skb data, > * but taking into account that pulling is expected to > * be very rare operation, it is worth to fight against > * further bloating skb head and crucify ourselves here instead. > * Pure masohism, indeed. 8)8) > */ > > /* Misery. We are in troubles, going to mincer fragments... */ > > > And since I cannot reproduce this here at the moment, I've decided not to > start with that. O.K. The BUG_ON() should at least catch other drivers hitting the same issue, and hopefully a search engine will point to this discussion. However, i suggest you post the fix to netdev, and see what others think. Andrew
The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.
To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
After more than 24 hours of continued uptime without any further
incidents I am giving this the seal of approval:
Tested-by: Felix Kaechele<felix@kaechele.ca>
Thanks again!
Felix
diff --git a/net/batman-adv/fragmentation.c b/net/batman-adv/fragmentation.c index 0899a729..c120c7c6 100644 --- a/net/batman-adv/fragmentation.c +++ b/net/batman-adv/fragmentation.c @@ -475,6 +475,17 @@ int batadv_frag_send_packet(struct sk_buff *skb, goto free_skb; } + /* GRO might have added fragments to the fragment list instead of + * frags[]. But this is not handled by skb_split and must be + * linearized to avoid incorrect length information after all + * batman-adv fragments were created and submitted to the + * hard-interface + */ + if (skb_has_frag_list(skb) && __skb_linearize(skb)) { + ret = -ENOMEM; + goto free_skb; + } + /* Create one header to be copied to all fragments */ frag_header.packet_type = BATADV_UNICAST_FRAG; frag_header.version = BATADV_COMPAT_VERSION;