Re: [syzbot] linux-next test error: WARNING in set_peer

[Kees Cook]

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