[xen master] x86emul: fix UB multiplications in S/G handling

[patchbot@xxxxxxx]

commit 69e99b5d4b087cfe508b4776703c5a9380cfb6d8
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Aug 13 16:41:25 2024 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Aug 13 16:41:25 2024 +0200

    x86emul: fix UB multiplications in S/G handling
    
    The conversion of the shifts to multiplications by the commits tagged
    below still wasn't quite right: The multiplications (of signed values)
    can overflow, too. As of 298556c7b5f8 ("x86emul: correct 32-bit address
    handling for AVX2 gathers") signed multiplication wasn't necessary
    anymore, though: The necessary sign-extension (if any) will happen as
    well when using intermediate variables of unsigned long types, and
    excess address bits are chopped off by truncate_ea().
    
    Fixes: b6a907f8c83d ("x86emul: replace UB shifts")
    Fixes: 21de9680eb59 ("x86emul: replace further UB shifts")
    Oss-fuzz: 71138
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/arch/x86/x86_emulate/x86_emulate.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index a0deafbd3b..9025382670 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -6369,11 +6369,11 @@ x86_emulate(
         {
             if ( (vex.w ? mask.qw[i] : mask.dw[i]) < 0 )
             {
-                signed long idx = b & 1 ? index.qw[i] : index.dw[i];
+                unsigned long idx = b & 1 ? index.qw[i] : index.dw[i];
 
                 rc = ops->read(ea.mem.seg,
                                truncate_ea(ea.mem.off +
-                                           idx * (1 << state->sib_scale)),
+                                           (idx << state->sib_scale)),
                                (void *)mmvalp + i * op_bytes, op_bytes, ctxt);
                 if ( rc != X86EMUL_OKAY )
                 {
@@ -6489,14 +6489,14 @@ x86_emulate(
 
         for ( i = 0; op_mask; ++i )
         {
-            long idx = b & 1 ? index.qw[i] : index.dw[i];
+            unsigned long idx = b & 1 ? index.qw[i] : index.dw[i];
 
             if ( !(op_mask & (1 << i)) )
                 continue;
 
             rc = ops->read(ea.mem.seg,
                            truncate_ea(ea.mem.off +
-                                       idx * (1 << state->sib_scale)),
+                                       (idx << state->sib_scale)),
                            (void *)mmvalp + i * op_bytes, op_bytes, ctxt);
             if ( rc != X86EMUL_OKAY )
             {
@@ -6643,9 +6643,9 @@ x86_emulate(
 
         for ( i = 0; op_mask; ++i )
         {
-            long idx = (b & 1 ? index.qw[i]
-                              : index.dw[i]) * (1 << state->sib_scale);
-            unsigned long offs = truncate_ea(ea.mem.off + idx);
+            unsigned long idx = b & 1 ? index.qw[i] : index.dw[i];
+            unsigned long offs = truncate_ea(ea.mem.off +
+                                             (idx << state->sib_scale));
             unsigned int j, slot;
 
             if ( !(op_mask & (1 << i)) )
@@ -6663,11 +6663,10 @@ x86_emulate(
              */
             for ( j = (slot = i) + 1; j < n; ++j )
             {
-                long idx2 = (b & 1 ? index.qw[j]
-                                   : index.dw[j]) * (1 << state->sib_scale);
-
+                idx = b & 1 ? index.qw[j] : index.dw[j];
                 if ( (op_mask & (1 << j)) &&
-                     truncate_ea(ea.mem.off + idx2) == offs )
+                     truncate_ea(ea.mem.off +
+                                 (idx << state->sib_scale)) == offs )
                     slot = j;
             }
 
--
generated by git-patchbot for /home/xen/git/xen.git#master