[Xen-devel] [PATCH RFC] tools/kdd: avoid adversarial optimisation hazard

[Wei Liu <wei.liu2@xxxxxxxxxx>]

There have been two attempts to fix kdd build with gcc 8.1
(437e00fe and 2de2b10b), but building with gcc 8.1 32 bit non-debug
build still yields the same error as in 437e00fe.

Ian wrote about adversarial optimisation in [0], one of the key points
is that computing an out-of-bounds pointer is UB.

<quote>
1. Adversarial optimissation hazard:

   The compiler may reason as follows:
   It is not legal to compute an out-of-bounds pointer.
   Doing so is UB.
   Therefore  ((uint8_t *)&ctrl.c32) + offset
   (which is caclulated unconditionally)
   is within the stack object `ctrl'.
   Therefore offset <= sizeof(ctrl).

1a. The compiler can continue to reason:
   Suppose offset > sizeof(ctrl.c32) but offset + len <=
   sizeof(ctrl.c32).  Because len is limited to 32-bit
   that can only happen if offset is large enough to
   wrap when len is added.  But I know that offset <= 216.
   So this situation cannot exist.
   Therefore I can remove the check for offset and
   rely only on the check for offset + len.

1b. The compiler can continue to reason:
   So offset <= 216.  I can therefore check that
   offset <= sizeof(ctrl.c32) using an instruction sequence
   that only looks at the bottom byte of offset (which on
   some architectures might be faster).

1c. If sizeof(ctrl.c32) ever becomes the same as sizeof(ctrl),
   the compiler can remove both checks entirely.
</quote>

Eliminate that UB by using uintptr_t to avoid the compiler reaching
the conclusion that offset <= sizeof(ctrl).

0: <23252.34284.742794.562828@xxxxxxxxxxxxxxxxxxxxxxxx>

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
---
I don't follow the "calculated unconditionally" bit. Isn't the
calculation only done in the else branch?

This patch does make gcc happy though.

Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Cc: Tim Deegan <tim@xxxxxxx>
Cc: Marek Marczykowski <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
Cc: Jan Beulich <jbeulich@xxxxxxxx>
---
 tools/debugger/kdd/kdd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 5a019a0a0c..054506f7a7 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -695,7 +695,8 @@ static void kdd_handle_read_ctrl(kdd_state *s)
             KDD_LOG(s, "Request outside of known control space\n");
             len = 0;
         } else {
-            memcpy(buf, ((uint8_t *)&ctrl.c32) + offset, len);
+            uintptr_t ptr_int = (uintptr_t)&ctrl.c32 + offset;
+            memcpy(buf, (uint8_t *)ptr_int, len);
         }
     }
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel