Re: [Qemu-devel] [RESEND] [PATCH] hw/9pfs: fix P9_STATS_GEN handling

["Kirill A. Shutemov" <kirill@shutemov.name>]

On Wed, Nov 06, 2013 at 09:41:47PM +0530, Aneesh Kumar K.V wrote:
> "Kirill A. Shutemov" <kirill@shutemov.name> writes:
> 
> > From: "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>
> >
> > Currently we have few issues with P9_STATS_GEN:
> >
> >  - We don't try to read st_gen anything except files or directories, but
> >    still set P9_STATS_GEN bit in st_result_mask. It may mislead client:
> >    we present garbage as valid st_gen.
> 
> We should return 0 right ? We do 
> 
>     memset(v9lstat, 0, sizeof(*v9lstat));
> 
> in stat_to_v9stat_dotl

The right way is not set P9_STATS_GEN in st_result_mask if we don't know
it.

> >  - If we failed to get valid st_gen with ENOTTY, we ignore error, but
> >    still set P9_STATS_GEN bit in st_result_mask.
> 
> and have v9lstat.st_gen set to zero 

The same as above.

And if ioctl(fd, FS_IOC_GETVERSION, st_gen) fails, nobody specifies what
will be stored into st_gen, if any. We should not relay that fs will not
touch st_gen even if it sounds reasonable.

> 
> >
> >  - If we failed to get valid st_gen with any other errno, we fail
> >    getattr altogether. It's excessive: we block valid client use-cases,
> >    like chdir(2) to non-readable directory with execution bit set.
> >
> 
> Can you explain this in detail ? 

If you have following tree:
 
% mkdir testdir
% echo test > testdir/testfile
% chmod -r testdir
 
In normal environment it's usable: you can chdir(2) into it and read files
inside if you know its name:
 
% cd testdir
% cat testfile
test
 
You only can't list directory content:
 
% ls
ls: cannot open directory .: Permission denied
 
With current qemu 9p implementation you'll get on guest -EACCES on chdir(2)
or read, since qemu fill fail to provide basic stats to guess. It happens
because qemu try open(2) non-readable file to run FS_IOC_GETVERSION and
fails getattr altogether.
 
I believe it also breaks more trivial use-cases: ls -l on non-readable
file or directory for the same reason. But I haven't checked that.

-- 
 Kirill A. Shutemov