linux-kernel - Re: [PATCH v15 2/4] syscall user dispatch: untag selector addresses before access

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZCYP+4gRZDqC0lRo@arm.com>
Date:   Thu, 30 Mar 2023 23:40:59 +0100
From:   Catalin Marinas <catalin.marinas@....com>
To:     Gregory Price <gourry.memverge@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-arch@...r.kernel.org, oleg@...hat.com, avagin@...il.com,
        peterz@...radead.org, luto@...nel.org, krisman@...labora.com,
        tglx@...utronix.de, corbet@....net, shuah@...nel.org,
        arnd@...db.de, Gregory Price <gregory.price@...verge.com>
Subject: Re: [PATCH v15 2/4] syscall user dispatch: untag selector addresses
 before access_ok

On Thu, Mar 30, 2023 at 05:21:22PM -0400, Gregory Price wrote:
> diff --git a/kernel/entry/syscall_user_dispatch.c b/kernel/entry/syscall_user_dispatch.c
> index 22396b234854..16086226b41c 100644
> --- a/kernel/entry/syscall_user_dispatch.c
> +++ b/kernel/entry/syscall_user_dispatch.c
> @@ -87,7 +87,18 @@ static int task_set_syscall_user_dispatch(struct task_struct *task, unsigned lon
>  		if (offset && offset + len <= offset)
>  			return -EINVAL;
>  
> -		if (selector && !access_ok(selector, sizeof(*selector)))
> +		/*
> +		 * access_ok will clear memory tags for tagged addresses on tasks where
> +		 * memory tagging is enabled.  To enable a tracer to set a tracee's
> +		 * selector not in the same tagging state, the selector address must be
> +		 * untagged for access_ok, otherwise an untagged tracer will always fail
> +		 * to set a tagged tracee's selector.
> +		 *
> +		 * The result of this is that a tagged tracer may be capable of setting
> +		 * an invalid address, and the tracee will SIGSEGV on the next syscall.
> +		 * This is equivalent to a task setting a bad selector (selector=0x1).
> +		 */

I'd drop the last paragraph above. Even without tagged pointers, a tracer
can set an invalid address (as you already mentioned) but the phrasing
some implies (to me) that if we did it differently, the tracer would not be
able to set an invalid pointer.

Either way,

Reviewed-by: Catalin Marinas <catalin.marinas@....com>

-- 
Catalin