[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/3] virtiofsd: Check file type in lo_flush()
From: |
Vivek Goyal |
Subject: |
Re: [PATCH 3/3] virtiofsd: Check file type in lo_flush() |
Date: |
Fri, 11 Dec 2020 09:25:44 -0500 |
On Thu, Dec 10, 2020 at 08:14:31PM +0000, Dr. David Alan Gilbert wrote:
> * Vivek Goyal (vgoyal@redhat.com) wrote:
> > On Thu, Dec 10, 2020 at 08:03:03PM +0000, Dr. David Alan Gilbert wrote:
> > > * Vivek Goyal (vgoyal@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 <lersek@redhat.com>
> > > > Signed-off-by: Vivek Goyal <vgoyal@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 <lersek@redhat.com>
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
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);
- [PATCH 0/3] virtiofsd: Fix lo_flush() and inode->posix_lock init, Vivek Goyal, 2020/12/07
- [PATCH 3/3] virtiofsd: Check file type in lo_flush(), Vivek Goyal, 2020/12/07
- ceph + freeipa ubuntu/fedora common small bug, Harry G. Coin, 2020/12/10
- Re: ceph + freeipa ubuntu/fedora common small bug, Dr. David Alan Gilbert, 2020/12/11
- Re: ceph + freeipa ubuntu/fedora common small bug, Vivek Goyal, 2020/12/11
- Re: ceph + freeipa ubuntu/fedora common small bug, Harry Coin, 2020/12/12
[PATCH 1/3] virtiofsd: Set up posix_lock hash table for root inode, Vivek Goyal, 2020/12/07
[PATCH 2/3] virtiofsd: Disable posix_lock hash table if remote locks are not enabled, Vivek Goyal, 2020/12/07