Re: [PATCH 13/13] icmp6,dhcp6,dhcp: Ensure buffer space for SCM_TIMESTAMP

Re: [PATCH 13/13] icmp6,dhcp6,dhcp: Ensure buffer space for SCM_TIMESTAMP

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

Hi Andrew,

On 4/22/22 13:59, Andrew Zaborowski wrote:
> Apparently the kernel will include message ancillary elements like
> SCM_TIMESTAMP even if there isn't enough space for the entire element.
> Reserve enough buffer space and check we received the whole
> SCM_TIMESTAMP element.
> 
> Fixes: adf18f2bae67 ("icmp6: Use SO_TIMESTAMP")
> Fixes: cd678415a75c ("dhcp6: Set lease expiry based on frame reception times")
> Fixes: c78ad1bb6d7e ("dhcp: Set lease expiry based on frame reception times")
> ---
>   ell/dhcp-transport.c  | 6 ++++--
>   ell/dhcp6-transport.c | 8 +++++---
>   ell/icmp6.c           | 7 +++++--
>   3 files changed, 14 insertions(+), 7 deletions(-)
> 

Applied, thanks.

Regards,
-Denis

[PATCH 13/13] icmp6, dhcp6, dhcp: Ensure buffer space for SCM_TIMESTAMP

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 3667 bytes --]

Apparently the kernel will include message ancillary elements like
SCM_TIMESTAMP even if there isn't enough space for the entire element.
Reserve enough buffer space and check we received the whole
SCM_TIMESTAMP element.

Fixes: adf18f2bae67 ("icmp6: Use SO_TIMESTAMP")
Fixes: cd678415a75c ("dhcp6: Set lease expiry based on frame reception times")
Fixes: c78ad1bb6d7e ("dhcp: Set lease expiry based on frame reception times")
---
 ell/dhcp-transport.c  | 6 ++++--
 ell/dhcp6-transport.c | 8 +++++---
 ell/icmp6.c           | 7 +++++--
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/ell/dhcp-transport.c b/ell/dhcp-transport.c
index d73930b..52da2db 100644
--- a/ell/dhcp-transport.c
+++ b/ell/dhcp-transport.c
@@ -117,7 +117,7 @@ static bool _dhcp_default_transport_read_handler(struct l_io *io,
 	struct cmsghdr *cmsg;
 	struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
 	struct msghdr msg = {};
-	unsigned char control[32];
+	unsigned char control[32 + CMSG_SPACE(sizeof(struct timeval))];
 
 	msg.msg_name = &saddr;
 	msg.msg_namelen = sizeof(saddr);
@@ -165,7 +165,9 @@ static bool _dhcp_default_transport_read_handler(struct l_io *io,
 	for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL;
 					cmsg = CMSG_NXTHDR(&msg, cmsg)) {
 		if (cmsg->cmsg_level == SOL_SOCKET &&
-				cmsg->cmsg_type == SCM_TIMESTAMP) {
+				cmsg->cmsg_type == SCM_TIMESTAMP &&
+				cmsg->cmsg_len ==
+				CMSG_LEN(sizeof(struct timeval))) {
 			const struct timeval *tv = (void *) CMSG_DATA(cmsg);
 
 			timestamp = tv->tv_sec * L_USEC_PER_SEC + tv->tv_usec;
diff --git a/ell/dhcp6-transport.c b/ell/dhcp6-transport.c
index 04bf4fb..13545b8 100644
--- a/ell/dhcp6-transport.c
+++ b/ell/dhcp6-transport.c
@@ -61,7 +61,7 @@ static bool _dhcp6_default_transport_read_handler(struct l_io *io,
 	struct cmsghdr *cmsg;
 	struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
 	struct msghdr msg = {};
-	unsigned char control[32];
+	unsigned char control[32 + CMSG_SPACE(sizeof(struct timeval))];
 
 	msg.msg_iov = &iov;
 	msg.msg_iovlen = 1;
@@ -78,8 +78,10 @@ static bool _dhcp6_default_transport_read_handler(struct l_io *io,
 	for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL;
 					cmsg = CMSG_NXTHDR(&msg, cmsg)) {
 		if (cmsg->cmsg_level == SOL_SOCKET &&
-				cmsg->cmsg_type == SCM_TIMESTAMP) {
-			const struct timeval *tv = (void *) CMSG_DATA(cmsg);
+				cmsg->cmsg_type == SCM_TIMESTAMP &&
+				cmsg->cmsg_len ==
+				CMSG_LEN(sizeof(struct timeval))) {
+		const struct timeval *tv = (void *) CMSG_DATA(cmsg);
 
 			timestamp = tv->tv_sec * L_USEC_PER_SEC + tv->tv_usec;
 		}
diff --git a/ell/icmp6.c b/ell/icmp6.c
index 158cfa8..ab2fe9c 100644
--- a/ell/icmp6.c
+++ b/ell/icmp6.c
@@ -203,7 +203,8 @@ static int icmp6_send_router_solicitation(int s, const uint8_t mac[static 6])
 static int icmp6_receive(int s, void *buf, size_t buf_len, struct in6_addr *src,
 				uint64_t *out_timestamp)
 {
-	char c_msg_buf[CMSG_SPACE(sizeof(int))];
+	char c_msg_buf[CMSG_SPACE(sizeof(int)) +
+			CMSG_SPACE(sizeof(struct timeval))];
 	struct iovec iov = {
 		.iov_base = buf,
 		.iov_len = buf_len,
@@ -242,7 +243,9 @@ static int icmp6_receive(int s, void *buf, size_t buf_len, struct in6_addr *src,
 			if (hops != 255)
 				return -EMULTIHOP;
 		} else if (cmsg->cmsg_level == SOL_SOCKET &&
-				cmsg->cmsg_type == SCM_TIMESTAMP) {
+				cmsg->cmsg_type == SCM_TIMESTAMP &&
+				cmsg->cmsg_len ==
+				CMSG_LEN(sizeof(struct timeval))) {
 			const struct timeval *tv = (void *) CMSG_DATA(cmsg);
 
 			timestamp = tv->tv_sec * L_USEC_PER_SEC + tv->tv_usec;
-- 
2.32.0