From d29c8b615a69438f01e58c329b8e34747e008685 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Sep 2019 16:41:12 +0200 Subject: [PATCH] incoming: don't handle 0xFFFFFFFF timestamps special in n_dhcp4_incoming_query_u32() First of all, from the naming of n_dhcp4_incoming_query_u32() it is confusing to coerce 0xFFFFFFFF to zero. It should just return the plain value. Also note that n_dhcp4_incoming_query_u32() only has three callers: n_dhcp4_incoming_query_lifetime(), n_dhcp4_incoming_query_t1() and n_dhcp4_incoming_query_t2(). Looking further, those three functions only have one caller: n_dhcp4_incoming_get_timeouts(). Note how the code there already tries to handle UINT32_MAX and interprets it as infinity (UINT64_MAX). But as it was, UINT32_MAX never actually was returned. It seems that RFC [1] does not specially define the meanings of 0xFFFFFFFF and 0. It sounds reasonable to assume that 0 just means 0 lifetime, and 0xFFFFFFFF means infinity. On the other hand, compare this to systemd's code [2], which coerces 0 to 1. This does not seem right to me though. Note how systemd returns 0xFFFFFFFF as-is. Drop the special handling of 0xFFFFFFFF from n_dhcp4_incoming_query_u32(). It now just returns the plain value and it's up to n_dhcp4_incoming_get_timeouts() to make sense of that. This will fix behavior, so that 0xFFFFFFFF will be reported as infinity, and not as zero. [1] https://tools.ietf.org/html/rfc2132#section-9.2 [2] https://github.com/systemd/systemd/blob/68c2b5ddb1881c40201c1d86a7852dd5c5c06a76/src/libsystemd-network/sd-dhcp-lease.c#L553 --- shared/n-dhcp4/src/n-dhcp4-incoming.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/shared/n-dhcp4/src/n-dhcp4-incoming.c b/shared/n-dhcp4/src/n-dhcp4-incoming.c index 255da45845..e7234c0a52 100644 --- a/shared/n-dhcp4/src/n-dhcp4-incoming.c +++ b/shared/n-dhcp4/src/n-dhcp4-incoming.c @@ -365,10 +365,7 @@ static int n_dhcp4_incoming_query_u32(NDhcp4Incoming *message, uint8_t option, u memcpy(&be32, data, sizeof(be32)); - if (be32 == (uint32_t)-1) - *u32p = 0; - else - *u32p = ntohl(be32); + *u32p = ntohl(be32); return 0; }