From c8a09bbaeb696a562f1607165a9a1bed6cb0f420 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gr=C3=B6ber?= Date: Thu, 6 Nov 2025 14:50:15 +0100 Subject: [PATCH 1/3] 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 | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ src/if-options.h | 3 +++ src/if.c | 2 ++ 5 files changed, 87 insertions(+), 5 deletions(-) diff --git a/src/dhcpcd.c b/src/dhcpcd.c index 149c2a3d..13cc5de6 100644 --- 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->argc : ifp->ctx->argc, + ifp->argv ? ifp->argv : ifp->ctx->argv, + options); } static void @@ -1364,7 +1367,7 @@ if_reboot(struct interface *ifp, int argc, char **argv) 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, int action, int argc, char **argv, int oi) 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 @@ dhcpcd_handleargs(struct dhcpcd_ctx *ctx, struct fd_list *fd, } 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 @@ main(int argc, char **argv, char **envp) 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); diff --git a/src/dhcpcd.h b/src/dhcpcd.h index 5f7fed00..9464a902 100644 --- 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]; diff --git a/src/if-options.c b/src/if-options.c index 083449b9..158bd4e6 100644 --- a/src/if-options.c +++ b/src/if-options.c @@ -44,6 +44,7 @@ #include #include #include +#include #include "config.h" #include "common.h" @@ -2986,6 +2987,68 @@ add_options(struct dhcpcd_ctx *ctx, const char *ifname, 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) { diff --git a/src/if-options.h b/src/if-options.h index a92697e1..4ed688b4 100644 --- a/src/if-options.h +++ b/src/if-options.h @@ -322,4 +322,7 @@ int add_options(struct dhcpcd_ctx *, const char *, 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 diff --git a/src/if.c b/src/if.c index 9b5ac2be..05c681d7 100644 --- 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); } From a0c40519444c6f745d6850742b2d1d507698b097 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gr=C3=B6ber?= Date: Sun, 7 Dec 2025 16:27:52 +0100 Subject: [PATCH 2/3] Fix subtly inconsistent use of ifp vs ctx arguments --- src/dhcpcd.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/dhcpcd.c b/src/dhcpcd.c index 13cc5de6..d588e604 100644 --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -730,7 +730,7 @@ dhcpcd_initstate(struct interface *ifp, unsigned long long options) { dhcpcd_initstate1(ifp, - ifp->argc ? ifp->argc : ifp->ctx->argc, + ifp->argv ? ifp->argc : ifp->ctx->argc, ifp->argv ? ifp->argv : ifp->ctx->argv, options); } @@ -1429,8 +1429,7 @@ reconf_reboot(struct dhcpcd_ctx *ctx, int action, int argc, char **argv, int oi) if (ifp->argv) free_argv_copy(ifp->argv); ifp->argv = copy_argv(argc, argv); - if (ifp->argv) - ifp->argc = argc; + ifp->argc = ifp->argv ? argc : 0; run_preinit(ifp); dhcpcd_prestartinterface(ifp); From fde51c75bb8e35ce1861f67c6aea0eec22c3959f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Gr=C3=B6ber?= Date: Sun, 7 Dec 2025 16:43:38 +0100 Subject: [PATCH 3/3] Make reconf_reboot logic more understandable --- src/dhcpcd.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/dhcpcd.c b/src/dhcpcd.c index d588e604..ff247579 100644 --- a/src/dhcpcd.c +++ b/src/dhcpcd.c @@ -1402,27 +1402,36 @@ reload_config(struct dhcpcd_ctx *ctx) } static void -reconf_reboot(struct dhcpcd_ctx *ctx, int action, int argc, char **argv, int oi) +reconf_reboot(struct dhcpcd_ctx *ctx, + const int reboot_else_applyaddr, + const int argc, char **argv, + const int first_iface_arg) { int i; struct interface *ifp; + bool args_empty = argc == first_iface_arg; TAILQ_FOREACH(ifp, ctx->ifaces, next) { - for (i = oi; i < argc; i++) { + // Find ifp in iface args + for (i = first_iface_arg; i < argc; i++) { if (strcmp(ifp->name, argv[i]) == 0) break; } - if (oi != argc && i == argc) - continue; + + bool iface_found = i != argc; + + if (!args_empty && !iface_found) + continue; // try to find other ifaces + // Note: if args_empty affect all interfaces + if (ifp->active == IF_ACTIVE_USER) { - if (action) + if (reboot_else_applyaddr) if_reboot(ifp, argc, argv); #ifdef INET else ipv4_applyaddr(ifp); #endif - } else if (i != argc) { - /* iface wasnt found above -> it's new. start it. */ + } else if (iface_found) { ifp->active = IF_ACTIVE_USER; dhcpcd_initstate1(ifp, argc, argv, 0); // control cmd args @@ -1539,7 +1548,7 @@ dhcpcd_signal_cb(int sig, void *arg) reload_config(ctx); /* Preserve any options passed on the commandline * when we were started. */ - reconf_reboot(ctx, 1, ctx->argc, ctx->argv, + reconf_reboot(ctx, 1 /*1=if_reboot*/, ctx->argc, ctx->argv, ctx->argc - ctx->ifc); return; case SIGUSR1: