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.
This commit is contained in:
Simon Kelley
2024-08-19 21:38:37 +01:00
parent d7d682637d
commit 46288c7e90
2 changed files with 15 additions and 13 deletions

View File

@@ -398,6 +398,10 @@ void dhcp_packet(time_t now, int pxe_fd)
struct in_pktinfo *pkt; struct in_pktinfo *pkt;
msg.msg_control = control_u.control; msg.msg_control = control_u.control;
msg.msg_controllen = sizeof(control_u); 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); cmptr = CMSG_FIRSTHDR(&msg);
pkt = (struct in_pktinfo *)CMSG_DATA(cmptr); pkt = (struct in_pktinfo *)CMSG_DATA(cmptr);
pkt->ipi_ifindex = rcvd_iface_index; pkt->ipi_ifindex = rcvd_iface_index;

View File

@@ -46,7 +46,7 @@ int send_from(int fd, int nowild, char *packet, size_t len,
#endif #endif
char control6[CMSG_SPACE(sizeof(struct in6_pktinfo))]; char control6[CMSG_SPACE(sizeof(struct in6_pktinfo))];
} control_u; } control_u;
iov[0].iov_base = packet; iov[0].iov_base = packet;
iov[0].iov_len = len; iov[0].iov_len = len;
@@ -60,19 +60,18 @@ int send_from(int fd, int nowild, char *packet, size_t len,
if (!nowild) if (!nowild)
{ {
struct cmsghdr *cmptr; struct cmsghdr *cmptr = msg.msg_control = &control_u.align;
msg.msg_control = &control_u;
msg.msg_controllen = sizeof(control_u);
cmptr = CMSG_FIRSTHDR(&msg);
/* 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 (to->sa.sa_family == AF_INET)
{ {
#if defined(HAVE_LINUX_NETWORK) #if defined(HAVE_LINUX_NETWORK)
struct in_pktinfo p; struct in_pktinfo *p = (struct in_pktinfo *)CMSG_DATA(cmptr);;
p.ipi_ifindex = 0; p->ipi_ifindex = 0;
p.ipi_spec_dst = source->addr4; p->ipi_spec_dst = source->addr4;
msg.msg_controllen = CMSG_SPACE(sizeof(struct in_pktinfo)); 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_len = CMSG_LEN(sizeof(struct in_pktinfo));
cmptr->cmsg_level = IPPROTO_IP; cmptr->cmsg_level = IPPROTO_IP;
cmptr->cmsg_type = IP_PKTINFO; cmptr->cmsg_type = IP_PKTINFO;
@@ -86,11 +85,10 @@ int send_from(int fd, int nowild, char *packet, size_t len,
} }
else else
{ {
struct in6_pktinfo p; 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_ifindex = iface; /* Need iface for IPv6 to handle link-local addrs */
p.ipi6_addr = source->addr6; p->ipi6_addr = source->addr6;
msg.msg_controllen = CMSG_SPACE(sizeof(struct in6_pktinfo)); 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_len = CMSG_LEN(sizeof(struct in6_pktinfo));
cmptr->cmsg_type = daemon->v6pktinfo; cmptr->cmsg_type = daemon->v6pktinfo;
cmptr->cmsg_level = IPPROTO_IPV6; cmptr->cmsg_level = IPPROTO_IPV6;