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 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.
>
> I'm afraid I can't take in your patch before we understand why the code
> doesn't function as expected. The problem is likely to be somewhere
> else.
>
> You can inspect the files under /var/lib/xen (or the location configured
> by XenServer). The names are like userdata-d.X.$UUID.libxl-json.
>
> One theory is the upgrade left some stale libraries which your xl then
> used. Use ldd `which xl` to see which libraries it used. And make sure
> it uses the latest ones that come with the upgrade.
>
> Wei.

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