Received-SPF: pass (google.com: domain of linux-kernel+bounces-128667-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <1bea609b-0acb-4adf-902c-e7e94de40977@rivosinc.com>
Date: Tue, 2 Apr 2024 13:38:51 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: ARM SVE ABI: kernel dropping SVE/SME state on syscalls
To: Mark Rutland <mark.rutland@arm.com>
Cc: Will Deacon <will@kernel.org>, Andrew Waterman <andrew@sifive.com>,
 Palmer Dabbelt <palmer@rivosinc.com>, lkml <linux-kernel@vger.kernel.org>,
 Mark Brown <broonie@kernel.org>, Dave Martin <Dave.Martin@arm.com>,
 =?UTF-8?B?QmrDtnJuIFTDtnBlbA==?= <bjorn@rivosinc.com>
References: <bc21c53f-23b9-4596-b757-fc7707f91f24@rivosinc.com>
 <ZgxKXy6r0X7Whu5U@FVFF77S0Q05N>
Content-Language: en-US
From: Vineet Gupta <vineetg@rivosinc.com>
In-Reply-To: <ZgxKXy6r0X7Whu5U@FVFF77S0Q05N>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

+CC Bjorn

On 4/2/24 11:11, Mark Rutland wrote:
> On Wed, Mar 27, 2024 at 05:30:00PM -0700, Vineet Gupta wrote:
>> Hi Will, Marc,

Thx for the reply and apologies for fat-fingering your name above.

>> 1. The vector store instruction (in say bash) takes a page fault, enters
>> kernel.
>> 2. In PF return path, a SIGCHLD signal is pending (a bash sub-shell
>> which exited, likely on different cpu).
> At this point, surely you need to save the VTYPE into a sigframe before
> delivering the signal?

Yes we do.

>> 3. kernel resumes in userspace signal handler which ends up making an
>> rt_sigreturn syscall - and which as specified discards the V state (and
>> makes VTYPE reg invalid).
> The state is discarded at syscall entry, but rt_sigreturn() runs *after* the
> discard. If you saved the original VTYPE prior to delivering the signal, it
> should be able to restore it regardless of whether it'd be clobbered at syscall
> entry.
>
> Surely you *must* save/restore VTYPE in the signal frame? Otherwise the signal
> handler can't make any syscall whatsoever, or it's responsible for saving and
> restoring VTYPE in userspace, which doesn't seem right.

Indeed I later realized that sigreturn is special as it has its own
state to restore. The discard prior drops the state during signal
handler which is anyways transient / throw-away so doesn't hurt this
specific case.

>> 4. When sigreturn finally returns to original Vector store instruction,
>> invalid VTYPE triggers an Illegal instruction which causes a SIGILL (as
>> state was discarded above).
>>
>> So there is no way dropping syscall state would work here.
> As above, I don't think that's quite true. It sounds to me like that the actual
> bug is that you don't save+restore VTYPE in the signal frame?

We do, but there was indeed a different bug which Bjorn found, in
sigreturn V state restore where we were (re)clobbering the V state by
using V-regs in copy-from-user and returning back with that ill restored
state.

>> How do you guys handle this for SVE/SME ? One way would be to not do the
>> discard in rt_sigreturn codepath, but I don't see that - granted I'm not
>> too familiar with arch/arm64/*/**
> IIUC this works on arm64 because we'll save all the original state when we
> deliver the signal, then restore that state *after* entry to the rt_sigreturn()
> syscall.
>
> I can go dig into that tomorrow, but I don't see how this can work unless we
> save *all* state prior to delivering the signal, and restoring *all* that state
> from the sigframe.

You don't have to, Bjorn found the bug and he'll post a fix to lists soon.

Thx,
-Vineet