| Message ID | 20240710032814.104643-4-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for user-only munmap races | expand |
On Wed, 10 Jul 2024 at 04:29, Richard Henderson <richard.henderson@linaro.org> wrote: > > Use these in helper_dc_dva and the FEAT_MOPS routines to > avoid a race condition with munmap in another thread. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > target/arm/tcg/helper-a64.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c > index 0ea8668ab4..c60d2a7ec9 100644 > --- a/target/arm/tcg/helper-a64.c > +++ b/target/arm/tcg/helper-a64.c > @@ -928,6 +928,8 @@ uint32_t HELPER(sqrt_f16)(uint32_t a, void *fpstp) > > void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) > { > + uintptr_t ra = GETPC(); > + > /* > * Implement DC ZVA, which zeroes a fixed-length block of memory. > * Note that we do not implement the (architecturally mandated) > @@ -948,8 +950,6 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) > > #ifndef CONFIG_USER_ONLY > if (unlikely(!mem)) { > - uintptr_t ra = GETPC(); > - > /* > * Trap if accessing an invalid page. DC_ZVA requires that we supply > * the original pointer for an invalid page. But watchpoints require > @@ -971,7 +971,9 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) > } > #endif > > + set_helper_retaddr(ra); > memset(mem, 0, blocklen); > + clear_helper_retaddr(); > } > > void HELPER(unaligned_access)(CPUARMState *env, uint64_t addr, > @@ -1120,7 +1122,9 @@ static uint64_t set_step(CPUARMState *env, uint64_t toaddr, > } > #endif > /* Easy case: just memset the host memory */ > + set_helper_retaddr(ra); > memset(mem, data, setsize); > + clear_helper_retaddr(); > return setsize; > } > > @@ -1163,7 +1167,9 @@ static uint64_t set_step_tags(CPUARMState *env, uint64_t toaddr, > } > #endif > /* Easy case: just memset the host memory */ > + set_helper_retaddr(ra); > memset(mem, data, setsize); > + clear_helper_retaddr(); > mte_mops_set_tags(env, toaddr, setsize, *mtedesc); > return setsize; > } I was briefly uncertain about this, but since we restrict the size of the amount of memory we're setting to not cross a guest page, if the memset() does fault it should only be for the case of "the whole block went away", so you never get the situation of "successfully wrote a chunk and then faulted partway through, and we should have written the tags for the partial write and did not". So Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c index 0ea8668ab4..c60d2a7ec9 100644 --- a/target/arm/tcg/helper-a64.c +++ b/target/arm/tcg/helper-a64.c @@ -928,6 +928,8 @@ uint32_t HELPER(sqrt_f16)(uint32_t a, void *fpstp) void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) { + uintptr_t ra = GETPC(); + /* * Implement DC ZVA, which zeroes a fixed-length block of memory. * Note that we do not implement the (architecturally mandated) @@ -948,8 +950,6 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) #ifndef CONFIG_USER_ONLY if (unlikely(!mem)) { - uintptr_t ra = GETPC(); - /* * Trap if accessing an invalid page. DC_ZVA requires that we supply * the original pointer for an invalid page. But watchpoints require @@ -971,7 +971,9 @@ void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in) } #endif + set_helper_retaddr(ra); memset(mem, 0, blocklen); + clear_helper_retaddr(); } void HELPER(unaligned_access)(CPUARMState *env, uint64_t addr, @@ -1120,7 +1122,9 @@ static uint64_t set_step(CPUARMState *env, uint64_t toaddr, } #endif /* Easy case: just memset the host memory */ + set_helper_retaddr(ra); memset(mem, data, setsize); + clear_helper_retaddr(); return setsize; } @@ -1163,7 +1167,9 @@ static uint64_t set_step_tags(CPUARMState *env, uint64_t toaddr, } #endif /* Easy case: just memset the host memory */ + set_helper_retaddr(ra); memset(mem, data, setsize); + clear_helper_retaddr(); mte_mops_set_tags(env, toaddr, setsize, *mtedesc); return setsize; } @@ -1497,7 +1503,9 @@ static uint64_t copy_step(CPUARMState *env, uint64_t toaddr, uint64_t fromaddr, } #endif /* Easy case: just memmove the host memory */ + set_helper_retaddr(ra); memmove(wmem, rmem, copysize); + clear_helper_retaddr(); return copysize; } @@ -1572,7 +1580,9 @@ static uint64_t copy_step_rev(CPUARMState *env, uint64_t toaddr, * Easy case: just memmove the host memory. Note that wmem and * rmem here point to the *last* byte to copy. */ + set_helper_retaddr(ra); memmove(wmem - (copysize - 1), rmem - (copysize - 1), copysize); + clear_helper_retaddr(); return copysize; }
Use these in helper_dc_dva and the FEAT_MOPS routines to avoid a race condition with munmap in another thread. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- target/arm/tcg/helper-a64.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)