linux-kernel - Re: [PATCH v2] fs: Add VirtualBox guest shared folder (vboxsf) support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <02e61d96-75b0-cb29-ecb5-36f91f41695e@redhat.com>
Date:   Tue, 16 Jan 2018 11:15:15 +0100
From:   Hans de Goede <hdegoede@...hat.com>
To:     Al Viro <viro@...IV.linux.org.uk>
Cc:     Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Michael Thayer <michael.thayer@...cle.com>,
        Larry Finger <Larry.Finger@...inger.net>,
        Christoph Hellwig <hch@...radead.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH v2] fs: Add VirtualBox guest shared folder (vboxsf)
 support

Hi Al,

Thank you for the quick review.

On 15-01-18 23:36, Al Viro wrote:
> * what hch said about mount options.  Get rid of that before the thing
>    goes into mainline.

Ack.

> * all "inode = GET_F_DENTRY(f)->d_inode;" are complete BS; it's called
>    file_inode(f).  Kill GET_F_DENTRY() off and be done with that.

Ok, will fix.

> * your ->rename() can race with ->get_link().  Look at the place where
>    the former reassigns ->path and frees the old value and think what
>    happens if the latter is called just prior to that kfree().
> 
> * the same goes for sf_inode_revalidate() vs. rename().
> 
> * just what happens to ->path of inode when e.g. its grandparent
>    directory is renamed?

Can I summarize all 3 above with: Caching the path is a bad idea and
instead the code should always look up the name from e.g. file->f_path.dentry ?

> * AFAICS, you consider all negative dentries invalid.  Why do you even
>    hash them, then?

I'm afraid I'm not entirely following you. Note I've no experience with fs
code prior to this. Also I'm not the original author of this code, this
code started as part of the out-of-tree kernel modules used by the
VirtualBox guest-additions. I've been working on cleaning these modules up
and then mainlinging them (there are 3 of them, this is the last).

Note I believe that this code is based on the fs/hostfs code.

Can you reword your question keeping my lack of experience wrt fs code
in mind ?

Regards,

Hans