libxl parser question

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

libxl parser question

Goncalo Gomes
I'm currently working with xen-unstable changeset 25260:0f53540494f7
and trying to add a `vncviewer` type to the libxl_types.idl, but
having difficulties understanding the root of a failure that occurs
after I add my changes. Hopefully I enclose enough detail below, but
if not, I'm happy to provide more information.

I've created a small patch to add a 'vncviewer' bool type to the
configuration parser of xl and based these changes on the "localtime"
implementation part of c/s 25131, see below:

diff -r 0f53540494f7 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Fri May 04 11:18:48 2012 +0100
+++ b/tools/libxl/libxl_create.c        Sat May 05 21:27:41 2012 +0000
@@ -156,6 +156,7 @@ int libxl__domain_build_info_setdefault(
     if (b_info->target_memkb == LIBXL_MEMKB_DEFAULT)
         b_info->target_memkb = b_info->max_memkb;
 
+    libxl_defbool_setdefault(&b_info->vncviewer, false);
     libxl_defbool_setdefault(&b_info->localtime, false);
 
     libxl_defbool_setdefault(&b_info->disable_migrate, false);
diff -r 0f53540494f7 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl       Fri May 04 11:18:48 2012 +0100
+++ b/tools/libxl/libxl_types.idl       Sat May 05 21:27:41 2012 +0000
@@ -250,6 +250,7 @@ libxl_domain_build_info = Struct("domain
     ("video_memkb",     MemKB),
     ("shadow_memkb",    MemKB),
     ("rtc_timeoffset",  uint32),
+    ("vncviewer",       libxl_defbool),
     ("localtime",       libxl_defbool),
     ("disable_migrate", libxl_defbool),
     ("cpuid",           libxl_cpuid_policy_list),
diff -r 0f53540494f7 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Fri May 04 11:18:48 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Sat May 05 21:27:41 2012 +0000
@@ -728,6 +728,7 @@ static void parse_config_data(const char
     if (!xlu_cfg_get_long(config, "rtc_timeoffset", &l, 0))
         b_info->rtc_timeoffset = l;
 
+    xlu_cfg_get_defbool(config, "vncviewer", &b_info->localtime, 0);
     xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, 0);
 
     if (!xlu_cfg_get_long (config, "videoram", &l, 0))

The outcome of this patch when applied and compiled in is that xl
aborts during the parse_config_data() call in the create_domain().

# ./xl create win7
Parsing config file win7
Aborted

This abort is the `default` case in the switch at xl_cmdimpl.c:736,
which gets triggered from an erroneous b_info->type with a bogus value
of 0x84 (which is neither PV nor HVM.)

For reference, the backtrace is:

(gdb) bt
#0  0xb7fe2416 in __kernel_vsyscall ()
#1  0xb7e28781 in *__GI_raise (sig=6) at
../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0xb7e2bbb2 in *__GI_abort () at abort.c:92
#3  0x0804fe4b in parse_config_data (configfile_filename_report=<value
optimized out>, configfile_data=<value optimized out>,
configfile_len=250, d_config=0xbffff55c) at xl_cmdimpl.c:841
#4  0x080526ba in create_domain (dom_info=<value optimized out>) at
xl_cmdimpl.c:1646
#5  0x0805c098 in main_create (argc=2, argv=0xbffffcc8) at
xl_cmdimpl.c:3470
#6  0x0804cfe4 in main (argc=3, argv=0xbffffcc4) at xl.c:176

I've also attached the win7 config file which I'm using, in case this
is a side effect of something wrong in config file itself (I should
add that it's mostly copied from an example and it successfully
creates a win7 domain without my patch applied.)

Any guesses or suggestions how to troubleshoot this further would be
greatly appreciated.

Thanks,
Goncalo


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel

win7 (261 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: libxl parser question

Ian Campbell-10
On Sat, 2012-05-05 at 19:19 -0400, Goncalo Gomes wrote:
> diff -r 0f53540494f7 tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c  Fri May 04 11:18:48 2012 +0100
> +++ b/tools/libxl/xl_cmdimpl.c  Sat May 05 21:27:41 2012 +0000
> @@ -728,6 +728,7 @@ static void parse_config_data(const char
>      if (!xlu_cfg_get_long(config, "rtc_timeoffset", &l, 0))
>          b_info->rtc_timeoffset = l;
>  
> +    xlu_cfg_get_defbool(config, "vncviewer", &b_info->localtime, 0);
>      xlu_cfg_get_defbool(config, "localtime", &b_info->localtime, 0);

You have a small typo here (localtime instead of vncviewer), but that
won't effect the crux of this issue.

I've tried reproducing using your config file with the patch applied and
I can't.

> [...]
> This abort is the `default` case in the switch at xl_cmdimpl.c:736,
> which gets triggered from an erroneous b_info->type with a bogus value
> of 0x84 (which is neither PV nor HVM.)

I think it might be useful to sprinkle prints of b_info->type everywhere
from the call to libxl_domain_build_info_init_type up until this switch
statement to see if you can identify the line which is overwriting this
field. I couldn't spot it by eye but something in there is presumably
blowing off the end of a buffer or something similar.

You should probably also validate the initial value, which comes from
c_info->type, and if that is wrong trace it back to the place which sets
that initial value.

Ian.



_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: libxl parser question

Goncalo Gomes
On Sun, 06 May 2012, Ian Campbell wrote:

> I've tried reproducing using your config file with the patch applied and
> I can't.
>
> > [...]
> > This abort is the `default` case in the switch at xl_cmdimpl.c:736,
> > which gets triggered from an erroneous b_info->type with a bogus value
> > of 0x84 (which is neither PV nor HVM.)
>
> I think it might be useful to sprinkle prints of b_info->type everywhere
> from the call to libxl_domain_build_info_init_type up until this switch
> statement to see if you can identify the line which is overwriting this
> field. I couldn't spot it by eye but something in there is presumably
> blowing off the end of a buffer or something similar.
>
> You should probably also validate the initial value, which comes from
> c_info->type, and if that is wrong trace it back to the place which sets
> that initial value.

Running 'make install' after various attempts seems to have done it.

I was all along under the impression that the dynamic linker would
pick the libxenlight in the current dir, which is why I hadn't run
'make install' every so often, but did instead occasionally run 'make
clean all' Coming to think of it, if the DLD was to pick the library
from the current directory, a world of security bugs and pain would
ensue, but for some reason I kept expecting the behaviour to be that.

Thanks though!

Goncalo

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: libxl parser question

Ian Campbell-10
On Sun, 2012-05-06 at 22:23 +0100, Goncalo Gomes wrote:

> On Sun, 06 May 2012, Ian Campbell wrote:
>
> > I've tried reproducing using your config file with the patch applied and
> > I can't.
> >
> > > [...]
> > > This abort is the `default` case in the switch at xl_cmdimpl.c:736,
> > > which gets triggered from an erroneous b_info->type with a bogus value
> > > of 0x84 (which is neither PV nor HVM.)
> >
> > I think it might be useful to sprinkle prints of b_info->type everywhere
> > from the call to libxl_domain_build_info_init_type up until this switch
> > statement to see if you can identify the line which is overwriting this
> > field. I couldn't spot it by eye but something in there is presumably
> > blowing off the end of a buffer or something similar.
> >
> > You should probably also validate the initial value, which comes from
> > c_info->type, and if that is wrong trace it back to the place which sets
> > that initial value.
>
> Running 'make install' after various attempts seems to have done it.
>
> I was all along under the impression that the dynamic linker would
> pick the libxenlight in the current dir, which is why I hadn't run
> 'make install' every so often, but did instead occasionally run 'make
> clean all' Coming to think of it, if the DLD was to pick the library
> from the current directory, a world of security bugs and pain would
> ensue, but for some reason I kept expecting the behaviour to be that.

Yeah, "." is not normally in the default library lookup path for a
variety of security etc issues as you suspected.You can either use
LD_LIBRARY_PATH=stuff:other-stuff or just run make install.

Ian.


_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel
Reply | Threaded
Open this post in threaded view
|

Re: libxl parser question

George Dunlap-4
On Tue, May 8, 2012 at 11:55 AM, Ian Campbell <[hidden email]> wrote:
> Yeah, "." is not normally in the default library lookup path for a
> variety of security etc issues as you suspected.You can either use
> LD_LIBRARY_PATH=stuff:other-stuff or just run make install.

...or if you're on a .deb-based system, "make deb && dpkg -i
dist/xen-*.deb".  That makes a nice easy way to un-install as well.

 -George

_______________________________________________
Xen-devel mailing list
[hidden email]
http://lists.xen.org/xen-devel