Re: [PATCH v6 7/8] samples/landlock: Add capability and namespace restriction support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] landlock: sandbox configuration leak via stale envp
--

commit 89caad8ff02abf64d07b124b6bd1e3374e39181c
Author: Mickaël Salaün <mic@digikod.net>

    samples/landlock: Add capability and namespace restriction support

    Extend the sandboxer sample to demonstrate the new Landlock capability
    and namespace restriction features. LL_CAP takes a colon-delimited list
    of allowed capabilities. Numeric values are always accepted, and when
    libcap development files are available, capability names can also be used.

> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index 030583273f3f7..1362f08ff14b9 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c

[ ... ]

> @@ -597,6 +824,32 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  			~LANDLOCK_ACCESS_NET_CONNECT_SEND_UDP;
>  	}
>  
> +	/* Removes namespace handling if not set by a user. */
> +	if (!getenv(ENV_NS_NAME) && !getenv(ENV_NS_QUIET_NAME))
> +		ruleset_attr.handled_permissions &=
> +			~LANDLOCK_PERMISSION_NAMESPACE_USE;
> +	if (!(ruleset_attr.handled_permissions &
> +	      LANDLOCK_PERMISSION_NAMESPACE_USE)) {
> +		unsetenv(ENV_NS_NAME);
> +		unsetenv(ENV_NS_QUIET_NAME);
> +	}
> +
> +#ifndef USE_LIBCAP
> +	if (getenv(ENV_CAP_NAME) || getenv(ENV_CAP_QUIET_NAME))
> +		fprintf(stderr,
> +			"Warning: Built without libcap; use capability numbers, not names.\n");
> +#endif
> +
> +	/* Removes capability handling if not set by a user. */
> +	if (!getenv(ENV_CAP_NAME) && !getenv(ENV_CAP_QUIET_NAME))
> +		ruleset_attr.handled_permissions &=
> +			~LANDLOCK_PERMISSION_CAPABILITY_USE;
> +	if (!(ruleset_attr.handled_permissions &
> +	      LANDLOCK_PERMISSION_CAPABILITY_USE)) {
> +		unsetenv(ENV_CAP_NAME);
> +		unsetenv(ENV_CAP_QUIET_NAME);
> +	}

[Severity: Medium]
Does this code leak the newly added LL_CAP and LL_NS environment variables
to the sandboxed child if environ is reallocated?

Looking at the end of main() in sandboxer.c, the child is executed with:

	execvpe(cmd_path, cmd_argv, envp);

The environment variables are scrubbed from the environment via unsetenv().
However, if any library (like one loaded via LD_PRELOAD) calls setenv() before
main() begins, libc will reallocate the global environ array onto the heap.

In this scenario, unsetenv() operates on the heap-allocated environ array,
while the local envp pointer passed to execvpe() still points to the original,
unmodified stack array containing the un-scrubbed sandbox configuration.

Could we use execvp(cmd_path, cmd_argv) or execvpe(cmd_path, cmd_argv, environ)
instead to ensure the properly scrubbed environment is propagated?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014082718.1435842-1-mic@digikod.net?part=7