netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ipsec-next v3] net: ipv6: fix field-spanning memcpy warning in AH output
@ 2025-08-12 15:51 Charalampos Mitrodimas
  2025-08-17 14:47 ` Steffen Klassert
  0 siblings, 1 reply; 2+ messages in thread
From: Charalampos Mitrodimas @ 2025-08-12 15:51 UTC (permalink / raw)
  To: Steffen Klassert, Herbert Xu, David S. Miller, David Ahern,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: netdev, linux-kernel, syzbot+01b0667934cdceb4451c,
	Charalampos Mitrodimas

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@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=01b0667934cdceb4451c
Signed-off-by: Charalampos Mitrodimas <charmitro@posteo.net>
---
Changes in v3:
- Properly separate address and extension header copying
- Add helper functions ah6_save_hdrs() and ah6_restore_hdrs() to avoid code duplication
- Update commit message to clarify this fixes a false positive warning, not a buffer overflow
- Target ipsec-next instead of net as this is a cleanup
- Remove unnecessary cast from memcpy() call
- Link to v2: https://lore.kernel.org/r/20250727-ah6-buffer-overflow-v2-1-c7b5f0984565@posteo.net

Changes in v2:
- Link correct syzbot dashboard link in patch tags
- Link to v1: https://lore.kernel.org/r/20250727-ah6-buffer-overflow-v1-1-1f3e11fa98db@posteo.net
---
 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 eb474f0987ae016b9d800e9f83d70d73171b21d2..95372e0f1d216b2408313229f94a1b572afcabd4 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);

---
base-commit: b711733e89a3f84c8e1e56e2328f9a0fa5facc7c
change-id: 20250727-ah6-buffer-overflow-ff795b87398d

Best regards,
-- 
Charalampos Mitrodimas <charmitro@posteo.net>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH ipsec-next v3] net: ipv6: fix field-spanning memcpy warning in AH output
  2025-08-12 15:51 [PATCH ipsec-next v3] net: ipv6: fix field-spanning memcpy warning in AH output Charalampos Mitrodimas
@ 2025-08-17 14:47 ` Steffen Klassert
  0 siblings, 0 replies; 2+ messages in thread
From: Steffen Klassert @ 2025-08-17 14:47 UTC (permalink / raw)
  To: Charalampos Mitrodimas
  Cc: Herbert Xu, David S. Miller, David Ahern, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, netdev, linux-kernel,
	syzbot+01b0667934cdceb4451c

On Tue, Aug 12, 2025 at 03:51:25PM +0000, Charalampos Mitrodimas wrote:
> 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@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=01b0667934cdceb4451c
> Signed-off-by: Charalampos Mitrodimas <charmitro@posteo.net>

Applied, thanks a lot!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-17 14:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-12 15:51 [PATCH ipsec-next v3] net: ipv6: fix field-spanning memcpy warning in AH output Charalampos Mitrodimas
2025-08-17 14:47 ` Steffen Klassert

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).