* Vivek Goyal (vgo...@redhat.com) wrote: > On Thu, Dec 10, 2020 at 08:14:31PM +0000, Dr. David Alan Gilbert wrote: > > * Vivek Goyal (vgo...@redhat.com) wrote: > > > On Thu, Dec 10, 2020 at 08:03:03PM +0000, Dr. David Alan Gilbert wrote: > > > > * Vivek Goyal (vgo...@redhat.com) wrote: > > > > > Currently lo_flush() is written in such a way that it expects to > > > > > receive > > > > > a FLUSH requests on a regular file (and not directories). For example, > > > > > we call lo_fi_fd() which searches lo->fd_map. If we open directories > > > > > using opendir(), we keep don't keep track of these in lo->fd_map > > > > > instead > > > > > we keep them in lo->dir_map. So we expect lo_flush() to be called on > > > > > regular files only. > > > > > > > > > > Even linux fuse client calls FLUSH only for regular files and not > > > > > directories. So put a check for filetype and return EBADF if > > > > > lo_flush() is called on a non-regular file. > > > > > > > > > > Reported-by: Laszlo Ersek <ler...@redhat.com> > > > > > Signed-off-by: Vivek Goyal <vgo...@redhat.com> > > > > > --- > > > > > tools/virtiofsd/passthrough_ll.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/tools/virtiofsd/passthrough_ll.c > > > > > b/tools/virtiofsd/passthrough_ll.c > > > > > index 8ba79f503a..48a109d3f6 100644 > > > > > --- a/tools/virtiofsd/passthrough_ll.c > > > > > +++ b/tools/virtiofsd/passthrough_ll.c > > > > > @@ -1968,7 +1968,7 @@ static void lo_flush(fuse_req_t req, fuse_ino_t > > > > > ino, struct fuse_file_info *fi) > > > > > struct lo_data *lo = lo_data(req); > > > > > > > > > > inode = lo_inode(req, ino); > > > > > - if (!inode) { > > > > > + if (!inode || !S_ISREG(inode->filetype)) { > > > > > fuse_reply_err(req, EBADF); > > > > > > > > Does that need a lo_inode_put(lo, &inode) in the new case? > > > > > > Good catch. Yes if inode is valid but file type is not regular, we need > > > to put inode reference. > > > > > > Do you want me to post a new patch or you will like to take care of > > > it. > > > > OK, so if we make this : > > > > if (!inode) { > > fuse_reply_err(req, EBADF); > > return; > > } > > > > if (!S_ISREG(inode->filetype)) { > > lo_inode_put(lo_data(req), &inode); > > fuse_reply_err(req, EBADF); > > return; > > } > > > > (Untested) > > Hi Dave, > > Above looks good. For your convenience, I updated the patch and also > tested it by running blogbench and things look fine. > > Vivek > > Subject: virtiofsd: Check file type in lo_flush() > > Currently lo_flush() is written in such a way that it expects to receive > a FLUSH requests on a regular file (and not directories). For example, > we call lo_fi_fd() which searches lo->fd_map. If we open directories > using opendir(), we keep don't keep track of these in lo->fd_map instead > we keep them in lo->dir_map. So we expect lo_flush() to be called on > regular files only. > > Even linux fuse client calls FLUSH only for regular files and not > directories. So put a check for filetype and return EBADF if > lo_flush() is called on a non-regular file. > > Reported-by: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Vivek Goyal <vgo...@redhat.com>
OK< thanks Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> and queued. (I've dropped the Tested-by since it's had a bit of forceful rework). > --- > tools/virtiofsd/passthrough_ll.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c > =================================================================== > --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2020-12-11 > 09:00:28.787669761 -0500 > +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c 2020-12-11 > 09:03:38.239496505 -0500 > @@ -1973,6 +1973,12 @@ static void lo_flush(fuse_req_t req, fus > return; > } > > + if (!S_ISREG(inode->filetype)) { > + lo_inode_put(lo, &inode); > + fuse_reply_err(req, EBADF); > + return; > + } > + > /* An fd is going away. Cleanup associated posix locks */ > if (lo->posix_lock) { > pthread_mutex_lock(&inode->plock_mutex); -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK