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

[Robin Lee <robinlee.sysu@xxxxxxxxx>]

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.
>
> Wei.
-robin

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