Package: dhcpcd-base Version: 1:10.1.0-11 Severity: important Forwarded: https://github.com/NetworkConfiguration/dhcpcd/pull/548 X-Debbugs-Cc: [email protected]
Dear Maintainer, While working on dhcpcd<>ifupdown integration I found that interface UP/DOWN events (cable re-plugging) can clear options that were passed when asking the global manager (dhcpcd.service) to start the interface initially. This doesn't happen when using per-iface (as used by ifupdown=0.8.44) or per-iface-per-af. The attached patch fixes the issue and has more detailed rationale. My preferred design for future-proof ifupdown<>dhcpcd.service interoperability relies on this not being broken, I will be uploading an NMU (1:10.3.0-1.1) to DELAYED/5 soon so my pending ifupdown=0.9 upload will not regress or have to hack around this. Thanks, --Daniel FYI: Find nomenclature and discussion in https://lists.debian.org/debian-devel/2025/10/msg00201.html
From 112cc12b8dabef6ca41558016e97bfa03b71df0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gr=C3=B6ber?= <[email protected]> Date: Thu, 6 Nov 2025 14:50:15 +0100 Subject: manager: Fix loosing iface options on CARRIER When an interface (re-)gains carrier dhcpcd_handlecarrier() runs dhcpcd_initstate() to kick off profile re-selection. Previously this used args originally passed when starting the manager (ctx->argv). However interfaces started via the manager control interface (dhcpcd_initstate1() in dhcpcd_handleargs()) may be started with different args. For example if we start a manager with dhcpcd -M --inactive and then start only IPv4 on an interface with dhcpcd -4 iface0 a subsequent CARRIER event will reset the interface to what amounts to "default config + `-M --inactive`" which in this case will enable ipv6 also! To fix this we keep a copy of the arguments used to start an interface in the manager (dhcpcd_handleargs()) code path around around (ifp->argv). In the current implementation args passed for renew following the initial interface start will not be persisted. This causes the interface to reset to a state of "defaults + config + profile + start-cmdline". For example (continuing the scenario above) after enabling ipv6 with -n: $ dhcpcd -6 -n iface0 A subsequent CARRIER event will disable ipv6 again as the effective arguments remain `-4 iface0` as passed during interface start. Note the per-interface daemon code path wasn't affected as ctx->args already contains the interface start args. --- src/dhcpcd.c | 21 +++++++++++++---- src/dhcpcd.h | 3 +++ src/if-options.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++ src/if-options.h | 3 +++ src/if.c | 2 ++ 5 files changed, 84 insertions(+), 5 deletions(-) --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -729,7 +729,10 @@ static void dhcpcd_initstate(struct interface *ifp, unsigned long long options) { - dhcpcd_initstate1(ifp, ifp->ctx->argc, ifp->ctx->argv, options); + dhcpcd_initstate1(ifp, + ifp->argc ?: ifp->ctx->argc, + ifp->argv ?: ifp->ctx->argv, + options); } static void @@ -1364,7 +1367,7 @@ if_reboot(struct interface *ifp, int arg oldopts = ifp->options->options; #endif script_runreason(ifp, "RECONFIGURE"); - dhcpcd_initstate1(ifp, argc, argv, 0); + dhcpcd_initstate1(ifp, argc, argv, 0); // control or main argv #ifdef INET if (ifp->options->options & DHCPCD_DHCP) dhcp_reboot_newopts(ifp, oldopts); @@ -1419,8 +1422,16 @@ reconf_reboot(struct dhcpcd_ctx *ctx, in ipv4_applyaddr(ifp); #endif } else if (i != argc) { + /* iface wasnt found above -> it's new. start it. */ ifp->active = IF_ACTIVE_USER; - dhcpcd_initstate1(ifp, argc, argv, 0); + dhcpcd_initstate1(ifp, argc, argv, 0); // control cmd args + + if (ifp->argv) + free_argv_copy(ifp->argv); + ifp->argv = copy_argv(argc, argv); + if (ifp->argv) + ifp->argc = argc; + run_preinit(ifp); dhcpcd_prestartinterface(ifp); } @@ -1773,7 +1784,7 @@ dumperr: } reload_config(ctx); - /* XXX: Respect initial commandline options? */ + /* Respect control cmd options! */ reconf_reboot(ctx, do_reboot, argc, argv, oifind); return 0; } @@ -2680,7 +2691,7 @@ start_manager: TAILQ_FOREACH(ifp, ctx.ifaces, next) { if (ifp->active) - dhcpcd_initstate1(ifp, argc, argv, 0); + dhcpcd_initstate1(ifp, argc, argv, 0); // main argv } if_learnaddrs(&ctx, ctx.ifaces, &ifaddrs); if_freeifaddrs(&ctx, &ifaddrs); --- a/src/dhcpcd.h +++ b/src/dhcpcd.h @@ -85,6 +85,9 @@ struct interface { uint8_t ssid[IF_SSIDLEN]; unsigned int ssid_len; + int argc; + char **argv; + char profile[PROFILE_LEN]; struct if_options *options; void *if_data[IF_DATA_MAX]; --- a/src/if-options.c +++ b/src/if-options.c @@ -44,6 +44,7 @@ #include <string.h> #include <unistd.h> #include <time.h> +#include <assert.h> #include "config.h" #include "common.h" @@ -2986,6 +2987,65 @@ add_options(struct dhcpcd_ctx *ctx, cons return r; } +#define ARGV_COPY_MAGIC ((char *)0x5a54292d273f3d34) +/*^ intentional truncation on 32bit arches */ + +char **copy_argv(int argc, char **argv) +{ + int i; + size_t strslen = 0; + for (i = 0; i < argc; i++) { + strslen += strlen(argv[i]) + 1; + } + if (strslen == 0) // also handles argc < 0 + return NULL; + + unsigned nptrs = 1 + (unsigned)argc + 1; + size_t ptrslen = nptrs * sizeof(char *); + void *buf = malloc(ptrslen + strslen); + char **ptrs = buf; + if (!buf) + return NULL; + + ptrs[0] = ARGV_COPY_MAGIC; + ptrs[nptrs - 1] = NULL; + + if (argc == 0) + goto out; + + char *strsp = (char *)&ptrs[nptrs]; + for (i = 0; i < argc; i++) { + size_t len = strlcpy(strsp, argv[i], strslen); + if (len >= strslen) // truncated + goto err; + + ptrs[1 + i] = strsp; + + strsp += len + 1; + if (strslen < len + 1) + goto err; + strslen -= len + 1; + } + + assert(strslen == 0); + assert(ptrs[nptrs - 1] == NULL); +out: + return &ptrs[1]; + +err: + free(buf); + return NULL; +} + +void free_argv_copy(char **argv) +{ + assert(argv[-1] == ARGV_COPY_MAGIC); + if (argv[-1] != ARGV_COPY_MAGIC) { + logerrx("%s: invalid argv", __func__); + } else + free(&argv[-1]); +} + void free_options(struct dhcpcd_ctx *ctx, struct if_options *ifo) { --- a/src/if-options.h +++ b/src/if-options.h @@ -322,4 +322,7 @@ int add_options(struct dhcpcd_ctx *, con void free_dhcp_opt_embenc(struct dhcp_opt *); void free_options(struct dhcpcd_ctx *, struct if_options *); +char **copy_argv(int argc, char **argv); +void free_argv_copy(char **argv); + #endif --- a/src/if.c +++ b/src/if.c @@ -100,6 +100,8 @@ if_free(struct interface *ifp) #endif rt_freeif(ifp); free_options(ifp->ctx, ifp->options); + if (ifp->argv) + free_argv_copy(ifp->argv); free(ifp); }
signature.asc
Description: PGP signature
