Re: [PATCH 1/2] userns: don't drop the exec securebits

["Mickaël Salaün" <mic@digikod.net>]

On Mon, Oct 26, 2026 at 09:53:29PM +0100, Christian Brauner wrote:
> SECBIT_EXEC_RESTRICT_FILE and SECBIT_EXEC_DENY_INTERACTIVE tell script
> interpreters to only run what AT_EXECVE_CHECK allows and to refuse
> interactive input. A service manager sets them together with their lock
> bits so that nothing below it can turn them off again.
> 
> But set_cred_user_ns() resets all securebits to SECUREBITS_DEFAULT. So
> any task that can create a user namespace sheds the locked exec bits
> with a single unshare(CLONE_NEWUSER) and joining one with setns() does
> the same. The lock is pointless if anyone who may create a user
> namespace can drop it, and that is pretty much everyone.
> 
> Resetting the other bits is fine. They are about how capabilities are
> computed and a new user namespace starts out with a full set of
> capabilities anyway. The exec bits grant nothing though. They are
> restrictions a task placed on itself, just like no_new_privs which a
> user namespace doesn't touch either. So keep them and their locks.
> 
> Reproducer:
> 
>   // SPDX-License-Identifier: GPL-2.0
>   /* Lock the exec securebits, then create a user namespace and look at them again. */
>   #define _GNU_SOURCE
>   #include <sched.h>
>   #include <stdio.h>
>   #include <sys/prctl.h>
> 
>   #define SECBIT_EXEC_RESTRICT_FILE		(1 << 8)
>   #define SECBIT_EXEC_RESTRICT_FILE_LOCKED	(1 << 9)
>   #define SECBIT_EXEC_DENY_INTERACTIVE		(1 << 10)
>   #define SECBIT_EXEC_DENY_INTERACTIVE_LOCKED	(1 << 11)
> 
>   int main(void)
>   {
>   	int bits = SECBIT_EXEC_RESTRICT_FILE | SECBIT_EXEC_RESTRICT_FILE_LOCKED |
>   		   SECBIT_EXEC_DENY_INTERACTIVE | SECBIT_EXEC_DENY_INTERACTIVE_LOCKED;
> 
>   	if (prctl(PR_SET_SECUREBITS, bits, 0, 0, 0)) {
>   		perror("PR_SET_SECUREBITS");
>   		return 1;
>   	}
>   	printf("before: 0x%x\n", prctl(PR_GET_SECUREBITS, 0, 0, 0, 0));
>   	printf("clear locked bits: %s\n", prctl(PR_SET_SECUREBITS, 0, 0, 0, 0) ? "refused" : "ALLOWED");
> 
>   	if (unshare(CLONE_NEWUSER)) {
>   		perror("unshare(CLONE_NEWUSER)");
>   		return 1;
>   	}
>   	printf("after unshare(CLONE_NEWUSER): 0x%x\n", prctl(PR_GET_SECUREBITS, 0, 0, 0, 0));
>   	return 0;
>   }
> 
> On v7.1 this prints "before: 0xf00", "clear locked bits: refused" and
> "after unshare(CLONE_NEWUSER): 0x0". With this patch the last line
> reads 0xf00 as well.
> 
> Fixes: a0623b2a1d59 ("security: Add EXEC_RESTRICT_FILE and EXEC_DENY_INTERACTIVE securebits")
> Cc: stable@vger.kernel.org # v6.14+
> Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>

Thanks!

Reviewed-by: Mickaël Salaün <mic@digikod.net>

> ---
>  Documentation/userspace-api/check_exec.rst | 4 ++++
>  kernel/user_namespace.c                    | 5 ++++-
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/userspace-api/check_exec.rst b/Documentation/userspace-api/check_exec.rst
> index 05dfe3b56f71..0904810e1d8d 100644
> --- a/Documentation/userspace-api/check_exec.rst
> +++ b/Documentation/userspace-api/check_exec.rst
> @@ -99,6 +99,10 @@ This secure bit may be set by user session managers, service managers,
>  container runtimes, sandboxer tools...  Except for test environments, the
>  related ``SECBIT_EXEC_DENY_INTERACTIVE_LOCKED`` bit should also be set.
>  
> +Both secure bits and their locks are inherited across :manpage:`fork(2)` and
> +:manpage:`execve(2)`, and they are kept when a task creates or joins a user
> +namespace.
> +
>  Here is the expected behavior for a script interpreter according to combination
>  of any exec securebits:
>  
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 50c374ed4687..cd748e1cbace 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -46,8 +46,11 @@ static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
>  {
>  	/* Start with the same capabilities as init but useless for doing
>  	 * anything as the capabilities are bound to the new user namespace.
> +	 * The exec securebits grant nothing, they are restrictions the task
> +	 * placed on itself. Keep them and their locks.
>  	 */
> -	cred->securebits = SECUREBITS_DEFAULT;
> +	cred->securebits = SECUREBITS_DEFAULT |
> +		(cred->securebits & (SECURE_ALL_UNPRIVILEGED | SECURE_ALL_UNPRIVILEGED << 1));
>  	cred->cap_inheritable = CAP_EMPTY_SET;
>  	cred->cap_permitted = CAP_FULL_SET;
>  	cred->cap_effective = CAP_FULL_SET;
> 
> -- 
> 2.53.0
>