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

[Tim Deegan <tim@xxxxxxx>]

Hi,

Apologies for the delay.  Several of my other hats were on fire.

> > I suspect the address, from which offset is derived, is bounded. But I
> > haven't found the spec for KD.
> 
> I don’t think there is one.

Indeed not.  The official way to extend windbg &c is to write a plugin
that runs on the Windows machine where you run the debugger.

At 13:37 +0100 on 26 Jul (1532612265), Ian Jackson wrote:
> It's still very obscure becaause this test
> 
>         if (offset > sizeof ctrl.c32 || offset + len > sizeof ctrl.c32) {
> 
> depends critically on the size of offset, etc.
> 
> Is it not still possible that this test could be fooled ?  Suppose
> offset is 0xffffffff.  Then before the test, offset is 0xfffffd33.

This is > sizeof ctrl.c32.  But:

> This kind of reasoning is awful.  The code should be rewritten so that
> it is obvious that it won't go wrong.

Yes.  How about this (compile tested only, and I haven't checked the buggy
gcc versions):

diff --git a/tools/debugger/kdd/kdd.c b/tools/debugger/kdd/kdd.c
index 5a019a0a0c..64aacde1ee 100644
--- a/tools/debugger/kdd/kdd.c
+++ b/tools/debugger/kdd/kdd.c
@@ -687,11 +687,11 @@ static void kdd_handle_read_ctrl(kdd_state *s)
         }
     } else {
         /* 32-bit control-register space starts at 0x[2]cc, for 84 bytes */
-        uint32_t offset = addr;
-        if (offset > 0x200)
-            offset -= 0x200;
-        offset -= 0xcc;
-        if (offset > sizeof ctrl.c32 || offset + len > sizeof ctrl.c32) {
+        uint32_t offset = addr - 0xcc;
+        if (offset > sizeof ctrl.c32)
+            offset = addr - 0x2cc;
+        if (offset > sizeof ctrl.c32
+            || len > sizeof ctrl.c32 - offset) {
             KDD_LOG(s, "Request outside of known control space\n");
             len = 0;
         } else {


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