Re: [Xen-devel] [PATCH] libxl: make sure buffer is null-terminated in libxl_read_file_contents

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jun 28, 2018 at 10:27:33AM +0800, Robin Lee wrote:
> On Wed, Jun 27, 2018 at 8:29 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> > On Wed, Jun 27, 2018 at 07:37:42PM +0800, Robin Lee wrote:
> >> On Wed, Jun 27, 2018 at 7:24 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> >> > On Wed, Jun 27, 2018 at 07:08:02PM +0800, Robin Lee wrote:
> >> >> On Wed, Jun 27, 2018 at 6:58 PM, Wei Liu <wei.liu2@xxxxxxxxxx> wrote:
> >> >> > On Wed, Jun 27, 2018 at 09:13:11AM +0000, Robin Lee wrote:
> >> >> >> On XenServer 7.1.1, we start a vm with XAPI but attach a block 
> >> >> >> device with xl.
> >> >> >> We create an empty json config for the vm with the content "{}\n" 
> >> >> >> and then
> >> >> >> run 'xl block-attach':
> >> >> >>
> >> >> >>   #  xl block-attach 1 phy:/dev/loop0 xvdz w
> >> >> >>   libxl: error: libxl_json.c:950:libxl__json_parse: yajl error: 
> >> >> >> parse error: trailing garbage
> >> >> >>                                       {} K]
> >> >> >>                        (right here) ------^
> >> >> >>
> >> >> >>   libxl: error: libxl_json.c:1053:libxl__object_from_json: unable to 
> >> >> >> generate libxl__json_object from JSON representation of 
> >> >> >> libxl_domain_config.
> >> >> >>   libxl: error: libxl.c:1995:device_addrm_aocomplete: unable to add 
> >> >> >> device
> >> >> >>   libxl_device_disk_add failed.
> >> >> >>
> >> >> >> After investigation, we found the buffer returned from 
> >> >> >> libxl_read_file_contents
> >> >> >> is not null-terminated. But later in libxl__object_from_json, the 
> >> >> >> buffer is expected to
> >> >> >> be null-terminated. So parsing may exceeded the end of file and get 
> >> >> >> in to uninisialized
> >> >> >> momery area.
> >> >> >>
> >> >> >> Signed-off-by: Robin Lee <robinlee.sysu@xxxxxxxxx>
> >> >> >
> >> >> > I can't seem to be able to reproduce this in upstream xen. Which 
> >> >> > version
> >> >> > of Xen does XenServer 7.1.1 have? You can get that from the output of
> >> >> > `xl info` -- look for xen_{major, minor, extra}.
> >> >> I also met a strange case. We didn't see this problem with Xen 4.7.1
> >> >> that released with
> >> >> XenServer 7.1.1. But since a recently hotfix from XenServer that 
> >> >> upgraded Xen to
> >> >> 4.7.4, this problem then shows up.
> >> >>
> >> >> The version of yajl (yajl-2.0.4-4.el7.x86_64)  never changed.
> >> >
> >> > As far as I can tell, the stored json file already contains trailing 0,
> >> > even in 4.7.4. There is nothing interesting between 4.7.1 and 4.7.4 in
> >> > that area of code.
> >> In my situation, the json file is created with external program and 
> >> contains
> >> just "{}\n" and not trailing 0.
> >
> > Alright. In that case please append 0 to the file you created.
> >
> > The json files are considered to be internal to libxl.
> OK. I can conform that json file generated by xl contains a trailing 0.
> But that seems not a common design to rely on the trailing 0 inside a text 
> file.

I know it is confusing but you probably shouldn't treat libxl-json file
as a text file. :-)

This also means what you previous did worked by chance -- it probably
worked because there happened to be 0's after the loaded buffer.

Wei.

> >
> > Wei.
> -robin

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