Re: [Xen-devel] [PATCH V5 2/7] libxl_read_file_contents: add new entry to read sysfs file

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Chun Yan Liu writes ("Re: [PATCH V5 2/7] libxl_read_file_contents: add new 
entry to read sysfs file"):
> >>> On 6/25/2015 at 07:09 PM, in message
> <21899.57676.368102.982820@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson
> <Ian.Jackson@xxxxxxxxxxxxx> wrote: 
> > Chunyan Liu writes ("[PATCH V5 2/7] libxl_read_file_contents: add new 
> > entry> > > Add a new entry libxl_read_sysfs_file_contents to handle sysfs 
> > file 
> > > specially. It would be used in later pvusb work. 
> >  
> > I think this still fails to detect a situation where the file is 
> > unexpectedly longer than the requested size ? 
> 
> +            } else if (feof(f)) {
> +                if (rs < datalen && tolerate_shrinking_file) {
> +                    datalen = rs;
> +                } else {
> 
> If the file is bigger than the requested size, it will fall to this
> branch and report error.

I don't think this is true.  I just applied your patch to my copy of
staging to see what the code looks like and saw this:

    if (stab.st_size && data_r) {
        data = malloc(datalen);
        if (!data) goto xe;

        rs = fread(data, 1, datalen, f);
        if (rs != datalen) {
            if (ferror(f)) {
                LOGE(ERROR, "failed to read %s", filename);
                goto xe;
            } else if (feof(f)) {
                if (rs < datalen && tolerate_shrinking_file) {
                    datalen = rs;
                } else {
                    LOG(ERROR, "%s changed size while we were reading it",
                        filename);
                    goto xe;
                }
            } else {
                abort();
            }
        }
    }

So I think in the case of a sysfs file which is >4096 bytes:

 - stat will succeed and give st_size == 4096
 - fread(,,4096,) will read the first 4096 bytes and return 4096
 - rs == datalen, so we don't take the `errors and special
     cases' branch
 - we return success having read 4096 bytes

But please feel free to explain why I'm wrong.

Thanks,
Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel