From 46288c7e9034bf1b4f3dc6398a3cf1e61e66816e Mon Sep 17 00:00:00 2001
From: Simon Kelley <simon@thekelleys.org.uk>
Date: Mon, 19 Aug 2024 21:38:37 +0100
Subject: [PATCH] Tidy up parameters to sendmsg() syscall.

The msg_controllen field passed to sendmsg is computed using the
CMSG_SPACE(), which is correct, but CMSG_SPACE() can include
padding bytes at the end of the passed structure if they are required
to align subsequent structures in the buffer. Make sure these
bytes are zeroed to avoid passing uninitiased memory to the kernel,
even though it will never touch these bytes.

Also tidy up the mashalling code in send_from to use pointers to
the structure being filled out, rather than a temporary copy which
then gets memcpy()'d into place. The DHCP sendmsg code has always
worked like this.

Thanks to  Dominik Derigs for running Memcheck as submitting the
initial patch.
---
 src/dhcp.c    |  4 ++++
 src/forward.c | 24 +++++++++++-------------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/src/dhcp.c b/src/dhcp.c
index b65facd..2603c76 100644
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -398,6 +398,10 @@ void dhcp_packet(time_t now, int pxe_fd)
       struct in_pktinfo *pkt;
       msg.msg_control = control_u.control;
       msg.msg_controllen = sizeof(control_u);
+
+      /* alignment padding passed to the kernel should not be uninitialised. */
+      memset(&control_u, 0, sizeof(control_u));
+
       cmptr = CMSG_FIRSTHDR(&msg);
       pkt = (struct in_pktinfo *)CMSG_DATA(cmptr);
       pkt->ipi_ifindex = rcvd_iface_index;
diff --git a/src/forward.c b/src/forward.c
index 10e7496..d235ce3 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -46,7 +46,7 @@ int send_from(int fd, int nowild, char *packet, size_t len,
 #endif
     char control6[CMSG_SPACE(sizeof(struct in6_pktinfo))];
   } control_u;
-  
+      
   iov[0].iov_base = packet;
   iov[0].iov_len = len;
 
@@ -60,19 +60,18 @@ int send_from(int fd, int nowild, char *packet, size_t len,
   
   if (!nowild)
     {
-      struct cmsghdr *cmptr;
-      msg.msg_control = &control_u;
-      msg.msg_controllen = sizeof(control_u);
-      cmptr = CMSG_FIRSTHDR(&msg);
+      struct cmsghdr *cmptr = msg.msg_control = &control_u.align;
 
+      /* alignment padding passed to the kernel should not be uninitialised. */
+      memset(&control_u, 0, sizeof(control_u));
+      
       if (to->sa.sa_family == AF_INET)
 	{
 #if defined(HAVE_LINUX_NETWORK)
-	  struct in_pktinfo p;
-	  p.ipi_ifindex = 0;
-	  p.ipi_spec_dst = source->addr4;
+	  struct in_pktinfo *p = (struct in_pktinfo *)CMSG_DATA(cmptr);;
+	  p->ipi_ifindex = 0;
+	  p->ipi_spec_dst = source->addr4;
 	  msg.msg_controllen = CMSG_SPACE(sizeof(struct in_pktinfo));
-	  memcpy(CMSG_DATA(cmptr), &p, sizeof(p));
 	  cmptr->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo));
 	  cmptr->cmsg_level = IPPROTO_IP;
 	  cmptr->cmsg_type = IP_PKTINFO;
@@ -86,11 +85,10 @@ int send_from(int fd, int nowild, char *packet, size_t len,
 	}
       else
 	{
-	  struct in6_pktinfo p;
-	  p.ipi6_ifindex = iface; /* Need iface for IPv6 to handle link-local addrs */
-	  p.ipi6_addr = source->addr6;
+	  struct in6_pktinfo *p = (struct in6_pktinfo *)CMSG_DATA(cmptr);
+	  p->ipi6_ifindex = iface; /* Need iface for IPv6 to handle link-local addrs */
+	  p->ipi6_addr = source->addr6;
 	  msg.msg_controllen = CMSG_SPACE(sizeof(struct in6_pktinfo));
-	  memcpy(CMSG_DATA(cmptr), &p, sizeof(p));
 	  cmptr->cmsg_len = CMSG_LEN(sizeof(struct in6_pktinfo));
 	  cmptr->cmsg_type = daemon->v6pktinfo;
 	  cmptr->cmsg_level = IPPROTO_IPV6;