| Message ID | 20260227100943.284203-1-florian.schmaus@codasip.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix dirname() handling in run_ptests() | expand |
Hi Florian, Thanks for the patch, comments below, Can you refer me to the musl dirname(3) function that uses static allocation?. I reviewed [1] and found that modifying the same string anyway doing the assignment is needed. [1] https://git.musl-libc.org/cgit/musl/tree/src/misc/dirname.c On Fri, Feb 27, 2026 at 4:09 AM Florian Schmaus <florian.schmaus@codasip.com> wrote: > It is required to use the return value of dirname() instead of > assuming that dirname() modifies the input string. While certain > implementations of dirname(), for example glibc, *may* modify the > input string, this is not guaranteed behavior. Musl, on the other > hand, does not modify the input string but instead returns a pointer > to a statically allocated string. > > Previously, ptest-runner would fail on musl systems: > > $ ptest-runner busybox > START: ptest-runner > 2026-02-27T09:21 > BEGIN: /usr/lib/busybox/ptest/run-ptest > ERROR: Unable to chdir(/usr/lib/busybox/ptest/run-ptest), Not a directory > > ERROR: Exited from signal Killed (9) > DURATION: 0 > END: /usr/lib/busybox/ptest/run-ptest > 2026-02-27T09:21 > STOP: ptest-runner > TOTAL: 1 FAIL: 1 > > Signed-off-by: Florian Schmaus <florian.schmaus@codasip.com> > --- > utils.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/utils.c b/utils.c > index 6cf7705586bc..7614dce99bd8 100644 > --- a/utils.c > +++ b/utils.c > @@ -361,13 +361,19 @@ run_ptests(struct ptest_list *head, const struct > ptest_options opts, > > fprintf(fp, "START: %s\n", progname); > PTEST_LIST_ITERATE_START(head, p) > - char ptest_dir[PATH_MAX] = {'\0'}; > + char *ptest_dir; > + char run_ptest[PATH_MAX] = {'\0'}; > int pipefd_stdout[2] = {-1, -1}; > int pipefd_stderr[2] = {-1, -1}; > int pty[2] = {-1, -1}; > > - strcpy(ptest_dir, p->run_ptest); > - dirname(ptest_dir); > strcpy(ptest_dir, p->run_ptest); // This assumes ptest_dir and run_ptest are both PATH_MAX; perhaps improve with a secure version of strcpy? ptest_dir = dirname(ptest_dir); > + /* Copy, since dirname() may modify its input > buffer */ > + if (strlcpy(run_ptest, p->run_ptest, > sizeof(run_ptest)) >= sizeof(run_ptest)) { > + fprintf(fp, "ERROR: %s exceeds > PATH_MAX\n", p->run_ptest); > + rc = -1; > + goto ptest_list_fail1; > + } > + ptest_dir = dirname(run_ptest); > > if (pipe2(pipefd_stdout, 0) == -1) { > fprintf(fp, "ERROR: pipe2() failed with: > %s.\n", strerror(errno)); > -- > 2.52.0 > >
diff --git a/utils.c b/utils.c index 6cf7705586bc..7614dce99bd8 100644 --- a/utils.c +++ b/utils.c @@ -361,13 +361,19 @@ run_ptests(struct ptest_list *head, const struct ptest_options opts, fprintf(fp, "START: %s\n", progname); PTEST_LIST_ITERATE_START(head, p) - char ptest_dir[PATH_MAX] = {'\0'}; + char *ptest_dir; + char run_ptest[PATH_MAX] = {'\0'}; int pipefd_stdout[2] = {-1, -1}; int pipefd_stderr[2] = {-1, -1}; int pty[2] = {-1, -1}; - strcpy(ptest_dir, p->run_ptest); - dirname(ptest_dir); + /* Copy, since dirname() may modify its input buffer */ + if (strlcpy(run_ptest, p->run_ptest, sizeof(run_ptest)) >= sizeof(run_ptest)) { + fprintf(fp, "ERROR: %s exceeds PATH_MAX\n", p->run_ptest); + rc = -1; + goto ptest_list_fail1; + } + ptest_dir = dirname(run_ptest); if (pipe2(pipefd_stdout, 0) == -1) { fprintf(fp, "ERROR: pipe2() failed with: %s.\n", strerror(errno));
It is required to use the return value of dirname() instead of assuming that dirname() modifies the input string. While certain implementations of dirname(), for example glibc, *may* modify the input string, this is not guaranteed behavior. Musl, on the other hand, does not modify the input string but instead returns a pointer to a statically allocated string. Previously, ptest-runner would fail on musl systems: $ ptest-runner busybox START: ptest-runner 2026-02-27T09:21 BEGIN: /usr/lib/busybox/ptest/run-ptest ERROR: Unable to chdir(/usr/lib/busybox/ptest/run-ptest), Not a directory ERROR: Exited from signal Killed (9) DURATION: 0 END: /usr/lib/busybox/ptest/run-ptest 2026-02-27T09:21 STOP: ptest-runner TOTAL: 1 FAIL: 1 Signed-off-by: Florian Schmaus <florian.schmaus@codasip.com> --- utils.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)