On 11/01/2023 1:57 pm, Jan Beulich wrote: > Make HVM=y release build behavior prone against array overrun, by > (ab)using array_access_nospec(). This is in particular to guard against > e.g. SH_type_unused making it here unintentionally. > > Signed-off-by: Jan Beulich <[email protected]> > --- > v2: New. > > --- a/xen/arch/x86/mm/shadow/private.h > +++ b/xen/arch/x86/mm/shadow/private.h > @@ -27,6 +27,7 @@ > // been included... > #include <asm/page.h> > #include <xen/domain_page.h> > +#include <xen/nospec.h> > #include <asm/x86_emulate.h> > #include <asm/hvm/support.h> > #include <asm/atomic.h> > @@ -368,7 +369,7 @@ shadow_size(unsigned int shadow_type) > { > #ifdef CONFIG_HVM > ASSERT(shadow_type < ARRAY_SIZE(sh_type_to_size)); > - return sh_type_to_size[shadow_type]; > + return array_access_nospec(sh_type_to_size, shadow_type);
I don't think this is warranted. First, if the commit message were accurate, then it's a problem for all arrays of size SH_type_unused, yet you've only adjusted a single instance. Secondly, if it were reliably 16 then we could do the basically-free "type &= 15;" modification. (It appears my change to do this automatically hasn't been taken yet.), but we'll end up with lfence variation here. But the value isn't attacker controlled. shadow_type always comes from Xen's metadata about the guest, not the guest itself. So I don't see how this can conceivably be a speculative issue even in principle. ~Andrew
