Re: [PATCH v3] virtiofsd: prevent opening of special files (CVE-2020-35517)

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

On Wed, Jan 27, 2021 at 03:27:23PM +0100, Miklos Szeredi wrote:
> On Wed, Jan 27, 2021 at 3:14 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Wed, Jan 27, 2021 at 02:01:54PM +0100, Miklos Szeredi wrote:
> 
> > > The problem here is there can also be a race between the open and the
> > > subsequent lo_do_lookup().
> > >
> > > At this point it's probably enough to verify that fuse_entry_param
> > > refers to the same object as the fh (using fstat and comparing st_dev
> > > and st_ino).
> >
> > Can you describe the race in detail? FUSE_CREATE vs FUSE_OPEN?
> > FUSE_CREATE vs FUSE_CREATE?
> 
> A race between FUSE_CREATE and external modification:
> 
> VIRTIOFSD: lo_create() {
> VIRTIOFSD:     fd = open(foo, O_CREAT | O_EXCL)
> EXTERNAL:  unlink(foo)
> EXTERNAL:  open(foo, O_CREAT)
> VIRTIOFSD:     lo_do_lookup() {
> VIRTIOFSD:         newfd = open(foo, O_PATH | O_NOFOLLOW)
> 
> Nothing serious will happen, but there will be a discrepancy between
> the open file and the inode that it references.  I.e.  the following
> in the client will yield weird results:
> 
> open(foo, O_CREAT) -> fd
> sprintf(procname, "/proc/self/fd/%i", fd);
> open(procname, O_RDONLY) -> fd2
> write(fd, buf, bufsize)
> read(fd2, buf, bufsize)
> 
> This is probably not a security issue, more of a quality of
> implementation issue.

Thanks for explaining. This is related to consistency when the shared
directory is accessed by multiple systems (e.g. other guests or the
host). virtiofsd doesn't support consistency in that case yet.

Let's treat this as a separate issue.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]