Xen devel - Aug 2010 - [PATCH]: xl: poison data objects in auto-generated destructors

Increase the probability of blowing up badly up during any
use-after-destroy scenarios.

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r f68726cdf357 tools/libxl/gentypes.py
--- a/tools/libxl/gentypes.py	Thu Aug 19 18:24:12 2010 +0100
+++ b/tools/libxl/gentypes.py	Fri Aug 20 15:51:35 2010 +0100
@@ -145,15 +145,19 @@ if __name__ == ''__main__'':
 
 #include <stdint.h>
 #include <stdlib.h>
+#include <string.h>
 
 #include "libxl.h"
 
+#define LIBXL_DTOR_POISON 0xa5
+
 """ % " ".join(sys.argv))
 
     for ty in [t for t in types if t.destructor_fn is not None and
t.autogenerate_destructor]:
         f.write("void %s(%s *p)\n" % (ty.destructor_fn, ty.typename))
         f.write("{\n")
         f.write(libxl_C_type_destroy(ty, "p", True))
+        f.write("\tmemset(p, LIBXL_DTOR_POISON, sizeof(*p));\n")
         f.write("}\n")
         f.write("\n")
     f.close()



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2010-08-20 at 15:49 +0100, Gianni Tedesco (3P)
wrote:
> Increase the probability of blowing up badly up during any
> use-after-destroy scenarios.
> 
> Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
I''d suggest perhaps only for debug=y builds but we don''t seem
to
propagate that concept into tools/ and it''s probably not worth it
anyhow.

So:

Acked-by: Ian Campbell <ian.campbell@citrix.com>

> diff -r f68726cdf357 tools/libxl/gentypes.py
> --- a/tools/libxl/gentypes.py	Thu Aug 19 18:24:12 2010 +0100
> +++ b/tools/libxl/gentypes.py	Fri Aug 20 15:51:35 2010 +0100
> @@ -145,15 +145,19 @@ if __name__ == ''__main__'':
>  
>  #include <stdint.h>
>  #include <stdlib.h>
> +#include <string.h>
>  
>  #include "libxl.h"
>  
> +#define LIBXL_DTOR_POISON 0xa5
> +
>  """ % " ".join(sys.argv))
>  
>      for ty in [t for t in types if t.destructor_fn is not None and
t.autogenerate_destructor]:
>          f.write("void %s(%s *p)\n" % (ty.destructor_fn,
ty.typename))
>          f.write("{\n")
>          f.write(libxl_C_type_destroy(ty, "p", True))
> +        f.write("\tmemset(p, LIBXL_DTOR_POISON, sizeof(*p));\n")
>          f.write("}\n")
>          f.write("\n")
>      f.close()
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2010-08-20 at 16:05 +0100, Ian Campbell wrote:
> On Fri, 2010-08-20 at 15:49 +0100, Gianni Tedesco (3P) wrote:
> > Increase the probability of blowing up badly up during any
> > use-after-destroy scenarios.
> > 
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> 
> I''d suggest perhaps only for debug=y builds but we don''t
seem to
> propagate that concept into tools/ and it''s probably not worth it
> anyhow.
The other approach would be to teach valgrind about such things but not
sure if it has a concept of freeing-in-two-stages. Same could be said
about gc pointers but I suspect sparse or something would work well to
catch those ''return gc allocated pointers'' etc. bugs.
> So:
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
> 
> 
> > diff -r f68726cdf357 tools/libxl/gentypes.py
> > --- a/tools/libxl/gentypes.py	Thu Aug 19 18:24:12 2010 +0100
> > +++ b/tools/libxl/gentypes.py	Fri Aug 20 15:51:35 2010 +0100
> > @@ -145,15 +145,19 @@ if __name__ == ''__main__'':
> >  
> >  #include <stdint.h>
> >  #include <stdlib.h>
> > +#include <string.h>
> >  
> >  #include "libxl.h"
> >  
> > +#define LIBXL_DTOR_POISON 0xa5
> > +
> >  """ % " ".join(sys.argv))
> >  
> >      for ty in [t for t in types if t.destructor_fn is not None and
t.autogenerate_destructor]:
> >          f.write("void %s(%s *p)\n" % (ty.destructor_fn,
ty.typename))
> >          f.write("{\n")
> >          f.write(libxl_C_type_destroy(ty, "p", True))
> > +        f.write("\tmemset(p, LIBXL_DTOR_POISON,
sizeof(*p));\n")
> >          f.write("}\n")
> >          f.write("\n")
> >      f.close()
> > 
> > 
> 
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 20 Aug 2010, Ian Campbell wrote:
> On Fri, 2010-08-20 at 15:49 +0100, Gianni Tedesco (3P) wrote:
> > Increase the probability of blowing up badly up during any
> > use-after-destroy scenarios.
> > 
> > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> 
> I''d suggest perhaps only for debug=y builds but we don''t
seem to
> propagate that concept into tools/ and it''s probably not worth it
> anyhow.
Agreed. We probably just want to revert this patch right before the
release, it might be worth to write a note about it somewhere.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2010-08-20 at 17:19 +0100, Ian Campbell wrote:
> On Fri, 2010-08-20 at 17:17 +0100, Stefano Stabellini wrote:
> > On Fri, 20 Aug 2010, Ian Campbell wrote:
> > > On Fri, 2010-08-20 at 15:49 +0100, Gianni Tedesco (3P) wrote:
> > > > Increase the probability of blowing up badly up during any
> > > > use-after-destroy scenarios.
> > > > 
> > > > Signed-off-by: Gianni Tedesco
<gianni.tedesco@citrix.com>
> > > 
> > > I''d suggest perhaps only for debug=y builds but we
don''t seem to
> > > propagate that concept into tools/ and it''s probably not
worth it
> > > anyhow.
> > 
> > Agreed. We probably just want to revert this patch right before the
> > release, it might be worth to write a note about it somewhere.
> 
> There''s no way we''ll remember to do that ;-)
> 
> If we care (and I''m not sure how much of the toolstack code is
likely to
> be performance critical at this level) then I think the time would be
> better spent making debug=[yn] do something under tools/libx?
> 
> Ian.
Agreed. Also this type of stuff is nice to have in widespread testing
and bug tickling that "the idiot users out there" are so damn good at
:)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2010-08-20 at 17:17 +0100, Stefano Stabellini
wrote:
> On Fri, 20 Aug 2010, Ian Campbell wrote:
> > On Fri, 2010-08-20 at 15:49 +0100, Gianni Tedesco (3P) wrote:
> > > Increase the probability of blowing up badly up during any
> > > use-after-destroy scenarios.
> > > 
> > > Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>
> > 
> > I''d suggest perhaps only for debug=y builds but we
don''t seem to
> > propagate that concept into tools/ and it''s probably not
worth it
> > anyhow.
> 
> Agreed. We probably just want to revert this patch right before the
> release, it might be worth to write a note about it somewhere.
There''s no way we''ll remember to do that ;-)

If we care (and I''m not sure how much of the toolstack code is likely
to
be performance critical at this level) then I think the time would be
better spent making debug=[yn] do something under tools/libx?

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel