In find_free_or_expired_ip, get_lease and check_requested_ip we'd allow addresses up to and including server->end_ip meaning that in some situations the broadcast address might have been allowed as a valid client address. find_free_or_expired_ip would check that the address didn't end in .255 but that is only correct for a 24-bit netmask. Now set start_ip to be at least one address above the subnet address and end_ip to be at lesat one address below the broadcast address. --- ell/dhcp-server.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/ell/dhcp-server.c b/ell/dhcp-server.c index bdf43d7..a4e7002 100644 --- a/ell/dhcp-server.c +++ b/ell/dhcp-server.c @@ -971,8 +971,8 @@ LIB_EXPORT bool l_dhcp_server_start(struct l_dhcp_server *server) */ if (!server->start_ip) { server->start_ip = ntohl(server->address) + 1; - server->end_ip = ntohl(server->address) | - (~ntohl(server->netmask)); + server->end_ip = (ntohl(server->address) | + (~ntohl(server->netmask))) - 1; } else { if ((server->start_ip ^ ntohl(server->address)) & ntohl(server->netmask)) @@ -981,17 +981,29 @@ LIB_EXPORT bool l_dhcp_server_start(struct l_dhcp_server *server) if ((server->end_ip ^ ntohl(server->address)) & ntohl(server->netmask)) return false; - } - /* - * We skip over IPs ending in 0 or 255 when selecting a free address - * later on but make sure end_ip is not 0xffffffff so we can use - * "<= server->end_ip" safely in the loop condition. - */ - if ((server->end_ip & 0xff) == 255) - server->end_ip--; + /* + * Directly ensure the [start_ip, end_ip] range doesn't + * include the subnet address or the broadcast address so that + * we have fewer checks to make when selecting a free address + * from that range. Additionally this ensures end_ip is not + * 0xffffffff so we can use the condition "<= server->end_ip" + * safely on uint32_t values. + * In find_free_or_expired_ip we skip over IPs ending in .0 or + * .255 even for netmasks other than 24-bit just to avoid + * generating addresses that could look suspicious even if + * they're legal. We don't exclude these addresses when + * explicitly requested by the client, i.e. in + * check_requested_ip. + */ + if ((server->start_ip & (~ntohl(server->netmask))) == 0) + server->start_ip++; + + if ((server->end_ip | ntohl(server->netmask)) == 0xffffffff) + server->end_ip--; + } - if (server->start_ip > server->end_ip) + if (server->start_ip >= server->end_ip) return false; if (!server->ifname) { -- 2.30.2