diff mbox series

[v3] dhcp: add settable max attempts, fix timeout overflow to zero

Message ID 20240118150818.49012-1-prestwoj@gmail.com (mailing list archive)
State Accepted, archived
Headers show
Series [v3] dhcp: add settable max attempts, fix timeout overflow to zero | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-ci-makedistcheck success Make Distcheck
prestwoj/iwd-ci-build success Build - Configure
prestwoj/iwd-ci-makecheck success Make Check
prestwoj/iwd-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-ci-clang success clang PASS
prestwoj/iwd-ci-testrunner success test-runner PASS

Commit Message

James Prestwood Jan. 18, 2024, 3:08 p.m. UTC
If DHCP is in a SELECTING/REQUESTING state and the number of attempts
reached a value where 2 << attempts overflowed an unsigned int the
next timeout would become zero, causing DHCP to never retry without
any event notification.

The number of attempts will now be configurable, but limited to
between 3 and 30 which prevents the bit shift from overflowing
'next_timeout' and provides some ultimate threshold where DHCP will
fail instead of try indefinitely.

A new event was added (L_DHCP_CLIENT_EVENT_MAX_ATTEMPTS_REACHED) to
notify netconfig of the situation. Netconfig will then send a failure
event and the consumer can decide how to proceed.

Fixes: f130c448 ("dhcp: Introduce timeout fuzzing")
---
 ell/dhcp.c      | 35 ++++++++++++++++++++++++++++++++---
 ell/dhcp.h      |  3 +++
 ell/ell.sym     |  1 +
 ell/netconfig.c |  6 ++++++
 4 files changed, 42 insertions(+), 3 deletions(-)

v3:
 * Added settable limit for attempts
 * Renamed "retries" to "attempts" for better clarity. "retries" implies
   some limit _after_ the first attmempt where "attempts" is much more
   clear in that it includes the first attempt as part of the limit.
 * DHCP is now limited to 30 attempts by default.

Comments

Denis Kenzior Jan. 19, 2024, 3:14 a.m. UTC | #1
Hi James,

On 1/18/24 09:08, James Prestwood wrote:
> If DHCP is in a SELECTING/REQUESTING state and the number of attempts
> reached a value where 2 << attempts overflowed an unsigned int the
> next timeout would become zero, causing DHCP to never retry without
> any event notification.
> 
> The number of attempts will now be configurable, but limited to
> between 3 and 30 which prevents the bit shift from overflowing
> 'next_timeout' and provides some ultimate threshold where DHCP will
> fail instead of try indefinitely.
> 
> A new event was added (L_DHCP_CLIENT_EVENT_MAX_ATTEMPTS_REACHED) to
> notify netconfig of the situation. Netconfig will then send a failure
> event and the consumer can decide how to proceed.
> 
> Fixes: f130c448 ("dhcp: Introduce timeout fuzzing")
> ---
>   ell/dhcp.c      | 35 ++++++++++++++++++++++++++++++++---
>   ell/dhcp.h      |  3 +++
>   ell/ell.sym     |  1 +
>   ell/netconfig.c |  6 ++++++
>   4 files changed, 42 insertions(+), 3 deletions(-)
> 

Applied, thanks.

Regards,
-Denis
diff mbox series

Patch

diff --git a/ell/dhcp.c b/ell/dhcp.c
index ece3e23..7d14ede 100644
--- a/ell/dhcp.c
+++ b/ell/dhcp.c
@@ -45,6 +45,8 @@ 
 	client->state = (s)
 
 #define BITS_PER_LONG (sizeof(unsigned long) * 8)
+#define CLIENT_MAX_ATTEMPT_LIMIT 30
+#define CLIENT_MIN_ATTEMPT_LIMIT 3
 
 enum dhcp_state {
 	DHCP_STATE_INIT,
@@ -159,6 +161,7 @@  struct l_dhcp_client {
 	uint32_t rtnl_add_cmdid;
 	struct l_rtnl_address *rtnl_configured_address;
 	uint8_t attempt;
+	uint8_t max_attempts;
 	l_dhcp_client_event_cb_t event_handler;
 	void *event_data;
 	l_dhcp_destroy_cb_t event_destroy;
@@ -558,9 +561,16 @@  static void dhcp_client_timeout_resend(struct l_timeout *timeout,
 		 * "The retransmission delay SHOULD be doubled with subsequent
 		 * retransmissions up to a maximum of 64 seconds.
 		 */
-		client->attempt += 1;
-		next_timeout = minsize(2 << client->attempt, 64);
-		break;
+		if (client->attempt < client->max_attempts) {
+			next_timeout = minsize(2 << client->attempt++, 64);
+			break;
+		}
+
+		CLIENT_DEBUG("Max request/discover attempts reached");
+
+		dhcp_client_event_notify(client,
+				L_DHCP_CLIENT_EVENT_MAX_ATTEMPTS_REACHED);
+		return;
 	case DHCP_STATE_INIT:
 	case DHCP_STATE_INIT_REBOOT:
 	case DHCP_STATE_REBOOTING:
@@ -988,6 +998,7 @@  LIB_EXPORT struct l_dhcp_client *l_dhcp_client_new(uint32_t ifindex)
 
 	client->state = DHCP_STATE_INIT;
 	client->ifindex = ifindex;
+	client->max_attempts = CLIENT_MAX_ATTEMPT_LIMIT;
 
 	/* Enable these options by default */
 	dhcp_enable_option(client, L_DHCP_OPTION_SUBNET_MASK);
@@ -1309,3 +1320,21 @@  LIB_EXPORT bool l_dhcp_client_set_rtnl(struct l_dhcp_client *client,
 	client->rtnl = rtnl;
 	return true;
 }
+
+LIB_EXPORT bool l_dhcp_client_set_max_attempts(struct l_dhcp_client *client,
+						uint8_t attempts)
+{
+	if (unlikely(!client))
+		return false;
+
+	if (unlikely(client->state != DHCP_STATE_INIT))
+		return false;
+
+	if (attempts < CLIENT_MIN_ATTEMPT_LIMIT ||
+				attempts > CLIENT_MAX_ATTEMPT_LIMIT)
+		return false;
+
+	client->max_attempts = attempts;
+
+	return true;
+}
diff --git a/ell/dhcp.h b/ell/dhcp.h
index 6ce4dde..3d15f94 100644
--- a/ell/dhcp.h
+++ b/ell/dhcp.h
@@ -41,6 +41,7 @@  enum l_dhcp_client_event {
 	L_DHCP_CLIENT_EVENT_LEASE_EXPIRED,
 	L_DHCP_CLIENT_EVENT_LEASE_RENEWED,
 	L_DHCP_CLIENT_EVENT_NO_LEASE,
+	L_DHCP_CLIENT_EVENT_MAX_ATTEMPTS_REACHED,
 };
 
 enum l_dhcp_server_event {
@@ -73,6 +74,8 @@  bool l_dhcp_client_set_hostname(struct l_dhcp_client *client,
 
 bool l_dhcp_client_set_rtnl(struct l_dhcp_client *client,
 					struct l_netlink *rtnl);
+bool l_dhcp_client_set_max_attempts(struct l_dhcp_client *client,
+					uint8_t attempts);
 
 const struct l_dhcp_lease *l_dhcp_client_get_lease(
 					const struct l_dhcp_client *client);
diff --git a/ell/ell.sym b/ell/ell.sym
index a887b2b..e12fd0e 100644
--- a/ell/ell.sym
+++ b/ell/ell.sym
@@ -255,6 +255,7 @@  global:
 	l_dhcp_client_set_rtnl;
 	l_dhcp_client_set_hostname;
 	l_dhcp_client_get_lease;
+	l_dhcp_client_set_max_attempts;
 	l_dhcp_client_start;
 	l_dhcp_client_stop;
 	l_dhcp_client_set_event_handler;
diff --git a/ell/netconfig.c b/ell/netconfig.c
index ab59299..06a717f 100644
--- a/ell/netconfig.c
+++ b/ell/netconfig.c
@@ -548,6 +548,12 @@  static void netconfig_dhcp_event_handler(struct l_dhcp_client *client,
 		if (!l_dhcp_client_start(nc->dhcp_client))
 			netconfig_failed(nc, AF_INET);
 
+		break;
+	case L_DHCP_CLIENT_EVENT_MAX_ATTEMPTS_REACHED:
+		L_WARN_ON(nc->v4_configured);
+
+		netconfig_failed(nc, AF_INET);
+
 		break;
 	}
 }