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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Aug 03, 2018 at 10:54:19AM +0100, Tim Deegan wrote:
> 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):

Yes the following diff works with the buggy compiler.

> 
> 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