Development discussion of WireGuard
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Jason@zx2c4.com
Cc: syzbot <syzbot+a448cda4dba2dac50de5@syzkaller.appspotmail.com>,
	davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	linux-kernel@vger.kernel.org, linux-next@vger.kernel.org,
	netdev@vger.kernel.org, pabeni@redhat.com, sfr@canb.auug.org.au,
	syzkaller-bugs@googlegroups.com, wireguard@lists.zx2c4.com
Subject: Re: [syzbot] linux-next test error: WARNING in set_peer
Date: Tue, 13 Sep 2022 17:55:39 -0700	[thread overview]
Message-ID: <202209131753.D1BDA803@keescook> (raw)
In-Reply-To: <00000000000060a74405e8945759@google.com>

On Tue, Sep 13, 2022 at 12:51:42PM -0700, syzbot wrote:
> memcpy: detected field-spanning write (size 28) of single field "&endpoint.addr" at drivers/net/wireguard/netlink.c:446 (size 16)

This is one way to fix it:

diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
index 0c0644e762e5..dbbeba216530 100644
--- a/drivers/net/wireguard/netlink.c
+++ b/drivers/net/wireguard/netlink.c
@@ -434,16 +434,16 @@ static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 	}
 
 	if (attrs[WGPEER_A_ENDPOINT]) {
-		struct sockaddr *addr = nla_data(attrs[WGPEER_A_ENDPOINT]);
+		struct endpoint *raw = nla_data(attrs[WGPEER_A_ENDPOINT]);
 		size_t len = nla_len(attrs[WGPEER_A_ENDPOINT]);
 
 		if ((len == sizeof(struct sockaddr_in) &&
-		     addr->sa_family == AF_INET) ||
+		     raw->addr.sa_family == AF_INET) ||
 		    (len == sizeof(struct sockaddr_in6) &&
-		     addr->sa_family == AF_INET6)) {
+		     raw->addr.sa_family == AF_INET6)) {
 			struct endpoint endpoint = { { { 0 } } };
 
-			memcpy(&endpoint.addr, addr, len);
+			memcpy(&endpoint.addrs, &raw->addrs, len);
 			wg_socket_set_peer_endpoint(peer, &endpoint);
 		}
 	}
diff --git a/drivers/net/wireguard/peer.h b/drivers/net/wireguard/peer.h
index 76e4d3128ad4..4fbe7940828b 100644
--- a/drivers/net/wireguard/peer.h
+++ b/drivers/net/wireguard/peer.h
@@ -19,11 +19,13 @@
 struct wg_device;
 
 struct endpoint {
-	union {
-		struct sockaddr addr;
-		struct sockaddr_in addr4;
-		struct sockaddr_in6 addr6;
-	};
+	struct_group(addrs,
+		union {
+			struct sockaddr addr;
+			struct sockaddr_in addr4;
+			struct sockaddr_in6 addr6;
+		};
+	);
 	union {
 		struct {
 			struct in_addr src4;


diffoscope shows the bounds check gets updated to the full union size:
│ -     cmp    $0x11,%edx
│ +     cmp    $0x1d,%edx

and the field name changes in the warning:
$ strings clang/drivers/net/wireguard/netlink.o.after | grep ^field
field "&endpoint.addrs" at drivers/net/wireguard/netlink.c:446


-- 
Kees Cook

  reply	other threads:[~2022-09-14  0:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-13 19:51 syzbot
2022-09-14  0:55 ` Kees Cook [this message]
2022-09-14 17:48   ` Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202209131753.D1BDA803@keescook \
    --to=keescook@chromium.org \
    --cc=Jason@zx2c4.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-next@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sfr@canb.auug.org.au \
    --cc=syzbot+a448cda4dba2dac50de5@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=wireguard@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).