Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 30 Mar 2023 23:40:59 +0100
From:   Catalin Marinas <catalin.marinas@arm.com>
To:     Gregory Price <gourry.memverge@gmail.com>
Cc:     linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
        linux-arch@vger.kernel.org, oleg@redhat.com, avagin@gmail.com,
        peterz@infradead.org, luto@kernel.org, krisman@collabora.com,
        tglx@linutronix.de, corbet@lwn.net, shuah@kernel.org,
        arnd@arndb.de, Gregory Price <gregory.price@memverge.com>
Subject: Re: [PATCH v15 2/4] syscall user dispatch: untag selector addresses
 before access_ok
Message-ID: <ZCYP+4gRZDqC0lRo@arm.com>
References: <20230330212121.1688-1-gregory.price@memverge.com>
 <20230330212121.1688-3-gregory.price@memverge.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230330212121.1688-3-gregory.price@memverge.com>
Precedence: bulk

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@arm.com>

-- 
Catalin