Re: [PATCH v2 2/3] seccomp: Allow upgrading to strict mode after enabling filters

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Stale syscall number is evaluated in the strict mode check after a ptracer modifies the syscall, potentially causing false positive kills.
- [High] Upgrading to strict mode from filter mode skips arch_seccomp_spec_mitigate, potentially leaving strict processes vulnerable to speculative execution flaws.
- [Medium] Missing `strict_upgrade` check in `seccomp_set_mode_filter` allows bypassed filter installation and TSYNC when seccomp is suspended.
--

commit 586f767ffd81c4f0c9b3df714ec047c0eabb4c30
Author: Jamie Hill-Daniel <clubby789@gmail.com>

seccomp: Allow upgrading to strict mode after enabling filters

Add a strict_upgrade flag to seccomp, which is enabled when
SECCOMP_MODE_STRICT is set after filters have been installed. This
will run the strict mode checks after all filters have been run.

This flag blocks thread syncing, in keeping with regular
SECCOMP_MODE_STRICT.

[ ... ]

> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 2d21cb6bbd94d..57ca0c49ec062 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c

[ ... ]

> @@ -1405,7 +1402,10 @@ int __secure_computing(void)
>  		__secure_computing_strict(this_syscall);  /* may call do_exit */
>  		return 0;
>  	case SECCOMP_MODE_FILTER:
> -		return __seccomp_filter(this_syscall, false);
> +		filter_ret = __seccomp_filter(this_syscall, false);
> +		if (current->seccomp.strict_upgrade)
> +			__secure_computing_strict(this_syscall);
> +		return filter_ret;

[Severity: High]
If a filter returns SECCOMP_RET_TRACE, a ptracer can change the syscall number.
While __seccomp_filter() correctly handles this by fetching the new syscall and
evaluating it against strict mode internally, this outer check in
__secure_computing() uses the original, unmodified this_syscall variable.

Does this redundant check use a stale this_syscall value, potentially killing
the process for a syscall that was already legitimately sanitized by the
ptracer?

>  	default:
>  		BUG();
>  	}

[ ... ]

> @@ -1420,30 +1420,23 @@ long prctl_get_seccomp(void)
>  /**
>   * seccomp_set_mode_strict: internal function for setting strict seccomp
>   *
> - * Once current->seccomp.mode is non-zero, it may not be changed.
> - *
> - * Returns 0 on success or -EINVAL on failure.
> + * If current->seccomp.mode is SECCOMP_MODE_FILTER, this will enable
> + * strict_upgrade.
>   */
> -static long seccomp_set_mode_strict(void)
> +static void seccomp_set_mode_strict(void)
>  {
>  	const unsigned long seccomp_mode = SECCOMP_MODE_STRICT;
> -	long ret = -EINVAL;
> -
>  	spin_lock_irq(&current->sighand->siglock);
>  
> -	if (!seccomp_may_assign_mode(seccomp_mode))
> -		goto out;
> -
>  #ifdef TIF_NOTSC
>  	disable_TSC();
>  #endif
> -	seccomp_assign_mode(current, seccomp_mode, 0);
> -	ret = 0;
> +	if (current->seccomp.mode == SECCOMP_MODE_FILTER)
> +		current->seccomp.strict_upgrade = 1;
> +	else
> +		seccomp_assign_mode(current, seccomp_mode, 0);

[Severity: High]
When a process enters filter mode using the SECCOMP_FILTER_FLAG_SPEC_ALLOW
flag, it opts out of speculative execution mitigations.

By skipping seccomp_assign_mode() when upgrading from filter mode, are we
failing to retroactively enforce these mitigations (like SSBD/STIBP) which are
normally enforced by strict mode?

> -out:
>  	spin_unlock_irq(&current->sighand->siglock);
> -
> -	return ret;
>  }

[ ... ]

> @@ -2015,7 +2006,8 @@ static long seccomp_set_mode_filter(unsigned int flags,
>  
>  	spin_lock_irq(&current->sighand->siglock);
>  
> -	if (!seccomp_may_assign_mode(seccomp_mode))
> +	/* Reachable if PTRACE_O_SUSPEND_SECCOMP has been used */
> +	if (current->seccomp.mode == SECCOMP_MODE_STRICT)
>  		goto out;

[Severity: Medium]
When a process uses strict_upgrade, its mode remains SECCOMP_MODE_FILTER.

Since this check only verifies SECCOMP_MODE_STRICT, does it allow a suspended
process with strict_upgrade set to successfully append new filters or use
SECCOMP_FILTER_FLAG_TSYNC, violating the immutability of strict mode?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-seccomp-combined-v2-0-41687a943ae0@hill-daniel.co.uk?part=2