From 1f803caf9a1f9afd566b34fc29f1757f7291f772 Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20H=C3=A4rdeman?= Date: Mon, 6 Oct 2025 09:48:43 +0200 Subject: [PATCH] dhcpv4: don't copy reqopts around MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Ok, this is a bit weird, dhcpv4_handle_msg() passes reqopts to dhcpv4_lease(), which stashes a copy of the reqopts in the struct dhcp_assignment, and then dhcpv4_handle_msg() uses the stashed copy instead. The only reason a copy is made seems to be so that the requested options can later be reported via ubus (in "ubus call dhcp ipv4leases"), but it is hard to see a use-case for that (the DHCP server has already replied to the client by the time the lease is available over ubus) and we don't do anything similar for DHCPv6, so remove the extra copying. Signed-off-by: David Härdeman Link: https://github.com/openwrt/odhcpd/pull/278 Signed-off-by: Álvaro Fernández Rojas --- src/dhcpv4.c | 23 ++++++++--------------- src/odhcpd.h | 3 --- src/ubus.c | 8 -------- 3 files changed, 8 insertions(+), 26 deletions(-) diff --git a/src/dhcpv4.c b/src/dhcpv4.c index 6b24ae1..236dbcb 100644 --- a/src/dhcpv4.c +++ b/src/dhcpv4.c @@ -398,7 +398,7 @@ static struct dhcp_assignment* dhcpv4_lease(struct interface *iface, enum dhcpv4_msg msg, const uint8_t *mac, const uint32_t reqaddr, uint32_t *leasetime, const char *hostname, const size_t hostname_len, const bool accept_fr_nonce, bool *incl_fr_opt, - uint32_t *fr_serverid, const uint8_t *reqopts, const size_t reqopts_len) + uint32_t *fr_serverid) { struct dhcp_assignment *a = find_assignment_by_hwaddr(iface, mac); struct lease *l = config_find_lease_by_mac(mac); @@ -503,14 +503,6 @@ dhcpv4_lease(struct interface *iface, enum dhcpv4_msg msg, const uint8_t *mac, } } - if (reqopts_len > 0) { - a->reqopts = realloc(a->reqopts, reqopts_len); - if (a->reqopts) { - memcpy(a->reqopts, reqopts, reqopts_len); - a->reqopts_len = reqopts_len; - } - } - if (!(a->flags & OAF_BOUND)) { a->accept_fr_nonce = accept_fr_nonce; *incl_fr_opt = accept_fr_nonce; @@ -650,8 +642,10 @@ void dhcpv4_handle_msg(void *addr, void *data, size_t len, return; break; case DHCPV4_OPT_REQOPTS: - req_opts = opt->data; - req_opts_len = opt->len; + if (opt->len > 0) { + req_opts = opt->data; + req_opts_len = opt->len; + } break; case DHCPV4_OPT_USER_CLASS: if (iface->filter_class) { @@ -694,8 +688,7 @@ void dhcpv4_handle_msg(void *addr, void *data, size_t len, case DHCPV4_MSG_RELEASE: a = dhcpv4_lease(iface, req_msg, req->chaddr, req_addr, &req_leasetime, req_hostname, req_hostname_len, - req_accept_fr, &incl_fr_opt, &fr_serverid, - req_opts, req_opts_len); + req_accept_fr, &incl_fr_opt, &fr_serverid); break; case DHCPV4_MSG_INFORM: break; @@ -838,8 +831,8 @@ void dhcpv4_handle_msg(void *addr, void *data, size_t len, dhcpv4_put(&reply, &cursor, DHCPV4_OPT_DNSSERVER, 4 * iface->dhcpv4_dns_cnt, iface->dhcpv4_dns); - for (size_t opt = 0; a && opt < a->reqopts_len; opt++) { - switch (a->reqopts[opt]) { + for (size_t opt = 0; a && opt < req_opts_len; opt++) { + switch (req_opts[opt]) { case DHCPV4_OPT_NTPSERVER: dhcpv4_put(&reply, &cursor, DHCPV4_OPT_NTPSERVER, 4 * iface->dhcpv4_ntp_cnt, iface->dhcpv4_ntp); diff --git a/src/odhcpd.h b/src/odhcpd.h index 8d5fa19..7d26365 100644 --- a/src/odhcpd.h +++ b/src/odhcpd.h @@ -265,8 +265,6 @@ struct dhcp_assignment { unsigned int flags; uint32_t leasetime; char *hostname; - uint8_t *reqopts; - size_t reqopts_len; uint8_t hwaddr[6]; @@ -456,7 +454,6 @@ inline static void free_assignment(struct dhcp_assignment *a) a->dhcp_free_cb(a); free(a->hostname); - free(a->reqopts); free(a); } diff --git a/src/ubus.c b/src/ubus.c index 47282da..1a16726 100644 --- a/src/ubus.c +++ b/src/ubus.c @@ -50,14 +50,6 @@ static int handle_dhcpv4_leases(struct ubus_context *ctx, _unused struct ubus_ob blobmsg_add_string(&b, "hostname", (c->hostname) ? c->hostname : ""); blobmsg_add_u8(&b, "accept-reconf", c->accept_fr_nonce); - if (c->reqopts_len > 0) { - buf = blobmsg_alloc_string_buffer(&b, "reqopts", c->reqopts_len * 4 + 1); - for (size_t i = 0; i < c->reqopts_len; i++) - buf += snprintf(buf, 5, "%" PRIu8 ",", c->reqopts[i]); - buf[-1] = '\0'; - blobmsg_add_string_buffer(&b); - } - m = blobmsg_open_array(&b, "flags"); if (c->flags & OAF_BOUND) blobmsg_add_string(&b, NULL, "bound"); -- 2.30.2