Thanks for all the improvement suggestions. Yes, I leveraged an LLM to generate the initial code (open & honest) but I'm willing to keep refining it to whatever state upstream requires. Thank you for not dismissing it outright right away. (FWIW, the initial code was even worse so this is the product of me intervening and editing it, despite my lack of C expertise).
A few more attempts at convincing :) * musl also supports $ORIGIN [https://elixir.bootlin.com/musl/v1.2.5/source/ldso/dynlink.c#L911] so there seems to be strong convergence on the concept beyond glibc * ideologically, everything about a program should be portable easily from one environment to another (the goal of systems such as Nix). Userland has allowed this support in nearly ever-space and the DT_INTERP / shebang path from the kernel seems to be a missing gap. We also have the shebang patch ready (CC @[email protected] ) but we wanted to see the reception to this first. I will wait to update the patch based on your feedback and Sashiko's, if you are convinced just to avoid spamming us with more patch files :) Farid Zakaria On Tue, Jun 23, 2026 at 1:14 PM Kees Cook <[email protected]> wrote: > > On Sun, Jun 21, 2026 at 09:39:33PM -0700, Farid Zakaria wrote: > > Currently, standard ELF and ELF FDPIC loaders expect a fixed path to the > > dynamic linker/interpreter (PT_INTERP). However, for systems utilizing > > relocatable dynamic interpreters (such as Nix/store-based environments), > > hardcoding this path is inflexible and breaks binary portability. > > > > Introduce support for resolving the $ORIGIN placeholder in the ELF > > interpreter path. This maps the dynamic linker relative to the path > > of the binary being executed, matching user-space origin resolution. > > > > To avoid code duplication, implement a shared 'resolve_elf_interpreter()' > > helper in the VFS exec layer. For safety, limit detection strictly to > > the prefix string "$ORIGIN" to prevent complex parsing exploits. > > Does any other OS that implements ELF support also support $ORIGIN in > the loader? $ORIGIN isn't even part of the ELF spec at all and is a > glibc extension, IIUC. > > I'm not excited about path-based string manipulations as they end up > being racy, and mucking with loader path seems like we're inviting > trouble (since the _binary_ specifies setuid-ness), and we've seen > issues with $ORIGIN before, even strictly outside of the kernel: > https://seclists.org/fulldisclosure/2010/Oct/257 > > > [...] > > diff --git a/fs/exec.c b/fs/exec.c > > index b92fe7db1..0978ae613 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -2024,6 +2024,48 @@ static int __init init_fs_exec_sysctls(void) > > fs_initcall(init_fs_exec_sysctls); > > #endif /* CONFIG_SYSCTL */ > > > > +char *resolve_elf_interpreter(struct linux_binprm *bprm, const char > > *elf_interpreter) > > +{ > > + char *pathbuf, *path, *slash, *resolved; > > + > > + if (strncmp(elf_interpreter, "$ORIGIN", 7) != 0) { > > + char *ret = kstrdup(elf_interpreter, GFP_KERNEL); > > + > > + return ret ? ret : ERR_PTR(-ENOMEM); > > + } > > But even if we did take this, I really don't want to incur a universal > penalty on exec for it. This is doing a kmalloc+dup (and later kfree) > for all non-$ORIGIN execs. So even if this gets added, it needs to be > handled differently. > > I would probably say this helper should return a struct file * instead > and have a fast-path for the common case. I think a check for leading > '$' (if strncmp doesn't get inlined) would be okay here as far as > "incurring common performance cost"; that string is almost certainly > cache-hot. > > > + pathbuf = kmalloc(PATH_MAX, GFP_KERNEL); > > + if (!pathbuf) > > + return ERR_PTR(-ENOMEM); > > + > > + path = file_path(bprm->file, pathbuf, PATH_MAX); > > + if (IS_ERR(path)) { > > + kfree(pathbuf); > > + return (char *)path; > > + } > > This still just _feels_ like an info leak or a race condition to me. I > can't give a credible example, though. But it creeps me out. :) > (I note the blog post also says "and the shabang" and I get even more > creeped out about seeing that patch.) > > > + > > + slash = strrchr(path, '/'); > > + if (slash) { > > + if (slash == path) > > + *(slash + 1) = '\0'; > > This is not idiomatic string manipulation. > > > + else > > + *slash = '\0'; > > More readable, IMO, as: > > if (slash) > slash[1] = '\0'; > else > path = ""; > > But does this match the glibc resolution logic? i.e. should it be: > > if (strncmp(elf_interpreter, "$ORIGIN/", 8) != 0) > ... > if (!slash) > slash = path; > *slash = '\0'; > ... > resolved = kasprintf(GFP_KERNEL, "%s/%s", path, elf_interpreter + 8); > > (requires the trailing /) > > > + } else { > > + kfree(pathbuf); > > + char *ret = kstrdup(elf_interpreter, GFP_KERNEL); > > + > > + return ret ? ret : ERR_PTR(-ENOMEM); > > This is the same as the logic top of the function. This repetition smells > of the LLM. :) > > > + } > > + > > + resolved = kasprintf(GFP_KERNEL, "%s%s", path, elf_interpreter + 7); > > + kfree(pathbuf); > > + if (!resolved) > > + return ERR_PTR(-ENOMEM); > > + > > + return resolved; > > +} > > +EXPORT_SYMBOL(resolve_elf_interpreter); > > + > > #ifdef CONFIG_EXEC_KUNIT_TEST > > #include "tests/exec_kunit.c" > > #endif > > diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h > > index 2c77e383e..17419cd3d 100644 > > --- a/include/linux/binfmts.h > > +++ b/include/linux/binfmts.h > > @@ -150,4 +150,6 @@ extern ssize_t read_code(struct file *, unsigned long, > > loff_t, size_t); > > int kernel_execve(const char *filename, > > const char *const *argv, const char *const *envp); > > > > +char *resolve_elf_interpreter(struct linux_binprm *bprm, const char > > *elf_interpreter); > > + > > #endif /* _LINUX_BINFMTS_H */ > > -- > > 2.51.2 > > > > So, I guess, I'd like more convincing, but I'm very happy to see a KUnit > test included! > > -Kees > > -- > Kees Cook
