From: Kees Cook <kees@kernel.org>
To: Farid Zakaria <farid.m.zakaria@gmail.com>
Cc: brauner@kernel.org, viro@zeniv.linux.org.uk, jack@suse.cz,
shuah@kernel.org, linux-fsdevel@vger.kernel.org,
linux-mm@kvack.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] fs: support $ORIGIN in ELF interpreter paths
Date: Tue, 23 Jun 2026 13:14:34 -0700 [thread overview]
Message-ID: <202606231236.325C882A78@keescook> (raw)
In-Reply-To: <20260622043934.179879-2-farid.m.zakaria@gmail.com>
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
next prev parent reply other threads:[~2026-06-23 20:14 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-22 4:39 [PATCH 0/2] fs: support $ORIGIN in ELF interpreter paths Farid Zakaria
2026-06-22 4:39 ` [PATCH 1/2] " Farid Zakaria
2026-06-22 9:53 ` Jori Koolstra
2026-06-23 20:14 ` Kees Cook [this message]
2026-06-23 20:35 ` Farid Zakaria
2026-06-22 4:39 ` [PATCH 2/2] selftests/exec: add test suites for $ORIGIN interpreter resolution Farid Zakaria
2026-06-22 10:39 ` [PATCH 0/2] fs: support $ORIGIN in ELF interpreter paths Jan Kara
2026-06-22 17:15 ` Farid Zakaria
2026-06-22 21:08 ` John Ericson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202606231236.325C882A78@keescook \
--to=kees@kernel.org \
--cc=brauner@kernel.org \
--cc=farid.m.zakaria@gmail.com \
--cc=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=shuah@kernel.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.