lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20251025160905.3857885-245-sashal@kernel.org>
Date: Sat, 25 Oct 2025 11:57:56 -0400
From: Sasha Levin <sashal@...nel.org>
To: patches@...ts.linux.dev,
	stable@...r.kernel.org
Cc: Charalampos Mitrodimas <charmitro@...teo.net>,
	syzbot+01b0667934cdceb4451c@...kaller.appspotmail.com,
	Steffen Klassert <steffen.klassert@...unet.com>,
	Sasha Levin <sashal@...nel.org>,
	davem@...emloft.net,
	dsahern@...nel.org,
	netdev@...r.kernel.org
Subject: [PATCH AUTOSEL 6.17-5.4] net: ipv6: fix field-spanning memcpy warning in AH output

From: Charalampos Mitrodimas <charmitro@...teo.net>

[ Upstream commit 2327a3d6f65ce2fe2634546dde4a25ef52296fec ]

Fix field-spanning memcpy warnings in ah6_output() and
ah6_output_done() where extension headers are copied to/from IPv6
address fields, triggering fortify-string warnings about writes beyond
the 16-byte address fields.

  memcpy: detected field-spanning write (size 40) of single field "&top_iph->saddr" at net/ipv6/ah6.c:439 (size 16)
  WARNING: CPU: 0 PID: 8838 at net/ipv6/ah6.c:439 ah6_output+0xe7e/0x14e0 net/ipv6/ah6.c:439

The warnings are false positives as the extension headers are
intentionally placed after the IPv6 header in memory. Fix by properly
copying addresses and extension headers separately, and introduce
helper functions to avoid code duplication.

Reported-by: syzbot+01b0667934cdceb4451c@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=01b0667934cdceb4451c
Signed-off-by: Charalampos Mitrodimas <charmitro@...teo.net>
Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>
Signed-off-by: Sasha Levin <sashal@...nel.org>
---

LLM Generated explanations, may be completely bogus:

YES

Why this needs backport
- Fixes real runtime WARNINGS from FORTIFY_SOURCE that can escalate to
  kernel panics on systems with panic_on_warn. The warning cited by
  syzbot shows a cross-field memcpy detected at runtime in AH output
  paths.
- Impacts users of IPv6 IPsec AH (xfrm). Even if AH is less common than
  ESP, false-positive warnings in networking code are undesirable and
  can disrupt CI/fuzzing or production systems with strict warn
  handling.

What is wrong in current stable code
- In AH output, the code intentionally copies the saved addresses plus
  the extension headers by writing past the 16-byte IPv6 address field
  into the memory directly following the base IPv6 header. This is
  semantically correct for the packet layout but trips FORTIFY’s “field-
  spanning write” checks.
- Problematic restores in output paths (write beyond `in6_addr` field):
  - net/ipv6/ah6.c:304–310 writes `extlen` bytes into `&top_iph->saddr`
    or `&top_iph->daddr`, which FORTIFY sees as overflowing a single
    field.
  - net/ipv6/ah6.c:437–443 repeats the same pattern after synchronous
    hash calculation.
- Symmetric “save” path copies from a field address:
  - net/ipv6/ah6.c:383–386 copies from
    `&top_iph->saddr`/`&top_iph->daddr` into the temporary buffer. While
    reads don’t trigger the runtime write check, the pattern mirrors the
    flawed restore approach.

What the patch changes
- Introduces helpers to separate copying of addresses from copying of
  extension headers, eliminating cross-field writes:
  - ah6_save_hdrs(): saves `saddr` (when CONFIG_IPV6_MIP6) and `daddr`,
    then copies extension headers from `top_iph + 1` into the temporary
    buffer’s `hdrs[]`.
  - ah6_restore_hdrs(): restores `saddr` (when CONFIG_IPV6_MIP6) and
    `daddr`, then copies extension headers into `top_iph + 1`.
- Replaces the field-spanning memcpy sites with these helpers:
  - In ah6_output_done(), instead of writing `extlen` bytes into
    `&top_iph->saddr`/`&top_iph->daddr` (net/ipv6/ah6.c:304–310), it
    calls ah6_restore_hdrs() to:
    - write addresses field-by-field, then
    - write extension headers starting at `top_iph + 1`, i.e.,
      immediately after the IPv6 base header, avoiding cross-field
      writes.
  - In ah6_output(), instead of saving `extlen` bytes starting from
    `&top_iph->saddr`/`&top_iph->daddr` into the temp buffer
    (net/ipv6/ah6.c:383–386), it calls ah6_save_hdrs() to:
    - read addresses field-by-field, then
    - copy extension headers from `top_iph + 1`.
- Extent calculation is preserved. `extlen` is unchanged and still
  includes `sizeof(*iph_ext)` when there are IPv6 extension headers; the
  helpers correctly use `extlen - sizeof(*iph_ext)` to copy only the
  extension headers into/out of `hdrs[]`.

Why it’s safe
- No functional semantics change: the same data (addresses + extension
  headers) are preserved/restored, just via safe destinations/sources
  (`top_iph + 1` for headers, explicit fields for addresses) instead of
  a single field pointer spanning into adjacent memory.
- Scope is small and entirely contained to net/ipv6/ah6.c; only
  ah6_output() and ah6_output_done() are touched plus two local static
  inline helpers.
- Config guards are preserved: when CONFIG_IPV6_MIP6 is enabled, `saddr`
  is saved/restored explicitly; otherwise only `daddr` is handled,
  matching prior behavior.
- Interactions with ipv6_clear_mutable_options() are unchanged; data is
  saved before zeroing mutable options and restored afterward as before.
- No ABI/API changes; only internal copying strategy is refactored to
  avoid FORTIFY warnings.

Stable tree criteria
- Bugfix that affects users: Prevents runtime WARN splats (and possible
  panic_on_warn) in IPv6 AH output paths. The commit references a syzbot
  report, indicating real-world triggerability.
- Minimal risk: Localized refactor with straightforward memcpy
  target/source changes.
- No new features or architectural changes.
- Touches a networking security subsystem (xfrm/AH) but in a very
  controlled way.

Version/dependency considerations
- The runtime “cross-field memcpy” WARN was introduced by fortify
  changes (e.g., commit akin to “fortify: Add run-time WARN for cross-
  field memcpy()”). All stable kernels that include these FORTIFY
  runtime checks and the current AH layout will benefit.
- The code structure in this branch matches the pre-fix pattern; the
  helpers should apply cleanly around existing sites at
  net/ipv6/ah6.c:304–310, 383–386, 437–443.
- No external dependencies; helpers are file-local.

Conclusion
- This is a targeted, correctness/safety fix that removes disruptive
  false-positive warnings with negligible regression risk. It should be
  backported to stable kernels that carry FORTIFY cross-field memcpy
  checks and the current AH implementation.

 net/ipv6/ah6.c | 50 +++++++++++++++++++++++++++++++-------------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index eb474f0987ae0..95372e0f1d216 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -46,6 +46,34 @@ struct ah_skb_cb {
 
 #define AH_SKB_CB(__skb) ((struct ah_skb_cb *)&((__skb)->cb[0]))
 
+/* Helper to save IPv6 addresses and extension headers to temporary storage */
+static inline void ah6_save_hdrs(struct tmp_ext *iph_ext,
+				 struct ipv6hdr *top_iph, int extlen)
+{
+	if (!extlen)
+		return;
+
+#if IS_ENABLED(CONFIG_IPV6_MIP6)
+	iph_ext->saddr = top_iph->saddr;
+#endif
+	iph_ext->daddr = top_iph->daddr;
+	memcpy(&iph_ext->hdrs, top_iph + 1, extlen - sizeof(*iph_ext));
+}
+
+/* Helper to restore IPv6 addresses and extension headers from temporary storage */
+static inline void ah6_restore_hdrs(struct ipv6hdr *top_iph,
+				    struct tmp_ext *iph_ext, int extlen)
+{
+	if (!extlen)
+		return;
+
+#if IS_ENABLED(CONFIG_IPV6_MIP6)
+	top_iph->saddr = iph_ext->saddr;
+#endif
+	top_iph->daddr = iph_ext->daddr;
+	memcpy(top_iph + 1, &iph_ext->hdrs, extlen - sizeof(*iph_ext));
+}
+
 static void *ah_alloc_tmp(struct crypto_ahash *ahash, int nfrags,
 			  unsigned int size)
 {
@@ -301,13 +329,7 @@ static void ah6_output_done(void *data, int err)
 	memcpy(ah->auth_data, icv, ahp->icv_trunc_len);
 	memcpy(top_iph, iph_base, IPV6HDR_BASELEN);
 
-	if (extlen) {
-#if IS_ENABLED(CONFIG_IPV6_MIP6)
-		memcpy(&top_iph->saddr, iph_ext, extlen);
-#else
-		memcpy(&top_iph->daddr, iph_ext, extlen);
-#endif
-	}
+	ah6_restore_hdrs(top_iph, iph_ext, extlen);
 
 	kfree(AH_SKB_CB(skb)->tmp);
 	xfrm_output_resume(skb->sk, skb, err);
@@ -378,12 +400,8 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
 	 */
 	memcpy(iph_base, top_iph, IPV6HDR_BASELEN);
 
+	ah6_save_hdrs(iph_ext, top_iph, extlen);
 	if (extlen) {
-#if IS_ENABLED(CONFIG_IPV6_MIP6)
-		memcpy(iph_ext, &top_iph->saddr, extlen);
-#else
-		memcpy(iph_ext, &top_iph->daddr, extlen);
-#endif
 		err = ipv6_clear_mutable_options(top_iph,
 						 extlen - sizeof(*iph_ext) +
 						 sizeof(*top_iph),
@@ -434,13 +452,7 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
 	memcpy(ah->auth_data, icv, ahp->icv_trunc_len);
 	memcpy(top_iph, iph_base, IPV6HDR_BASELEN);
 
-	if (extlen) {
-#if IS_ENABLED(CONFIG_IPV6_MIP6)
-		memcpy(&top_iph->saddr, iph_ext, extlen);
-#else
-		memcpy(&top_iph->daddr, iph_ext, extlen);
-#endif
-	}
+	ah6_restore_hdrs(top_iph, iph_ext, extlen);
 
 out_free:
 	kfree(iph_base);
-- 
2.51.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ