| Message ID | 20220628042117.368549-38-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/arm: Scalable Matrix Extension | expand |
On Tue, 28 Jun 2022 at 05:50, Richard Henderson <richard.henderson@linaro.org> wrote: > > In parse_user_sigframe, the kernel rejects duplicate sve records, > or records that are smaller than the header. We were silently > allowing these cases to pass, dropping the record. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/aarch64/signal.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c > index 8b352abb97..8fbe98d72f 100644 > --- a/linux-user/aarch64/signal.c > +++ b/linux-user/aarch64/signal.c > @@ -318,10 +318,13 @@ static int target_restore_sigframe(CPUARMState *env, > break; > > case TARGET_SVE_MAGIC: > + if (sve || size < sizeof(struct target_sve_context)) { > + goto err; > + } > if (cpu_isar_feature(aa64_sve, env_archcpu(env))) { > vq = sve_vq(env); > sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16); > - if (!sve && size == sve_size) { > + if (size == sve_size) { > sve = (struct target_sve_context *)ctx; > break; > } On the other hand, the kernel seems to happily allow records which are larger than the SVE_SIG_CONTEXT_SIZE, whereas we ignore the record unless there's an exact size match. I notice the kernel has a bunch of signal frame test cases in tools/testing/selftests/arm64/signal/testcases -- do we pass those ? thanks -- PMM
On 7/4/22 17:38, Peter Maydell wrote: > I notice the kernel has a bunch of signal frame test > cases in tools/testing/selftests/arm64/signal/testcases -- > do we pass those ? Most but not all of them. The ones we don't pass are those for which SVE state has been discarded across a syscall and so the signal frame record is expected to be missing. I thought about fixing those, but decided not to do within this series. r~
On 7/4/22 17:38, Peter Maydell wrote: >> case TARGET_SVE_MAGIC: >> + if (sve || size < sizeof(struct target_sve_context)) { >> + goto err; >> + } >> if (cpu_isar_feature(aa64_sve, env_archcpu(env))) { >> vq = sve_vq(env); >> sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16); >> - if (!sve && size == sve_size) { >> + if (size == sve_size) { >> sve = (struct target_sve_context *)ctx; >> break; >> } > > On the other hand, the kernel seems to happily allow records > which are larger than the SVE_SIG_CONTEXT_SIZE, whereas we > ignore the record unless there's an exact size match. Yeah, this gets fixed properly in patch 39. Perhaps I should simply squash this with that? r~
On 7/5/22 09:00, Richard Henderson wrote: > On 7/4/22 17:38, Peter Maydell wrote: >>> case TARGET_SVE_MAGIC: >>> + if (sve || size < sizeof(struct target_sve_context)) { >>> + goto err; >>> + } >>> if (cpu_isar_feature(aa64_sve, env_archcpu(env))) { >>> vq = sve_vq(env); >>> sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16); >>> - if (!sve && size == sve_size) { >>> + if (size == sve_size) { >>> sve = (struct target_sve_context *)ctx; >>> break; >>> } >> >> On the other hand, the kernel seems to happily allow records >> which are larger than the SVE_SIG_CONTEXT_SIZE, whereas we >> ignore the record unless there's an exact size match. > > Yeah, this gets fixed properly in patch 39. > Perhaps I should simply squash this with that? Bah! No, those are two separate checks: the minimum size to contain vq and flags (target_sve_context) and the minimum size to contain all of the vector data (TARGET_SVE_SIG_CONTEXT_SIZE). The latter *is* fixed in patch 39, but this one should stay as-is. r~
diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c index 8b352abb97..8fbe98d72f 100644 --- a/linux-user/aarch64/signal.c +++ b/linux-user/aarch64/signal.c @@ -318,10 +318,13 @@ static int target_restore_sigframe(CPUARMState *env, break; case TARGET_SVE_MAGIC: + if (sve || size < sizeof(struct target_sve_context)) { + goto err; + } if (cpu_isar_feature(aa64_sve, env_archcpu(env))) { vq = sve_vq(env); sve_size = QEMU_ALIGN_UP(TARGET_SVE_SIG_CONTEXT_SIZE(vq), 16); - if (!sve && size == sve_size) { + if (size == sve_size) { sve = (struct target_sve_context *)ctx; break; }
In parse_user_sigframe, the kernel rejects duplicate sve records, or records that are smaller than the header. We were silently allowing these cases to pass, dropping the record. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/aarch64/signal.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)