Re: [PATCH] Ignore symlink traversal failures in grub-mount readdir

[Andrey Borzenkov]

В Sun, 10 Mar 2013 14:01:53 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <address@hidden> пишет:

> On 09.03.2013 17:47, Andrey Borzenkov wrote:
> 
> > В Fri, 12 Oct 2012 17:09:54 +0100
> > Colin Watson <address@hidden> пишет:
> > 
> >> This is very much a temporary hack, so I'm sending it here for
> >> discussion rather than just committing it even though it's quite simple.
> >>
> >> r3036.1.15 introduced support for filling in the attributes of files in
> >> fuse_readdir.  However, symlinks to directories are passed to call_fill
> >> with info.dir unset; call_fill will then try to open them with
> >> grub_file_open, and get GRUB_ERR_BAD_FILE_TYPE because it's ultimately a
> >> directory not a regular file.  It then causes the whole readdir call to
> >> fail.  The net effect is that if you, for example, have a symlink
> >> anywhere in the top level of a filesystem, then the entire filesystem
> >> appears empty in grub-mount.  This is the root cause of
> >> https://bugs.launchpad.net/bugs/1051306.
> >>
> >> I think that the proper solution is to pass the full
> >> grub_fshelp_filetype to dirhook functions, which would permit
> >> implementing true symlink support in grub-mount.  That would be a fairly
> >> large change that I don't have time for at the moment.  As a stopgap, I
> >> suggest that we ignore errors from individual grub_file_open calls
> >> during fuse_readdir.  How does this patch look?
> >>
> > 
> > The only reason to call grub_file_open() is to fetch file size, and file
> > size is already available when hooks are called. So what about patch
> > below? It fixes problem for me, and it trivial enough. This allows
> > directory listing to work again. I can extend it with info.is_link to
> > return proper filetype to FUSE, but implementing full support needs
> > adding readlink that is a separate topic.
> > 
> > I tested it with ext4 and cpio and it works. Testing on more systems
> > (in particular, NTFS, which is the only one with non-trivial change) is
> > appreciated.
> > 
> 
> Some time ago I made a similar patch but for another motivation: current
> code is way too inefficient for large directories as you have to rescan
> directory for every file. The problem with this patch is 10 bytes
> increase of core.img. This may be acceptable given this problem (it
> happens in ls on runtime as well) and inefficency of scanning.
> 

Any thoughts on it? I'm fine with either approach; there was some
reduction of core size in the meantime so may be slight increase is
acceptable?

signature.asc
Description: PGP signature