* [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
@ 2020-05-13 16:33 Fotis Xenakis
2020-05-15 3:48 ` Liu Bo
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Fotis Xenakis @ 2020-05-13 16:33 UTC (permalink / raw)
To: virtio-fs
Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
descriptor for the file to mmap() to QEMU (specifically, it passes the
file handle as requested from the guest).
This fixes it, using lo_fi_fd() to map the file handle to the right file
descriptor on the host.
Signed-off-by: Fotis Xenakis <foxen@windowslive.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 e09d557d74..44bb24eb5e 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2709,7 +2709,7 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
msg.flags[0] = vhu_flags;
if (fi) {
- fd = fi->fh;
+ fd = lo_fi_fd(req, fi);
} else {
res = asprintf(&buf, "%i", lo_fd(req, ino));
if (res == -1) {
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
2020-05-13 16:33 [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping Fotis Xenakis
@ 2020-05-15 3:48 ` Liu Bo
2020-05-15 16:50 ` Fotis Xenakis
2020-05-15 12:52 ` Vivek Goyal
2020-05-18 10:45 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 7+ messages in thread
From: Liu Bo @ 2020-05-15 3:48 UTC (permalink / raw)
To: Fotis Xenakis; +Cc: virtio-fs
On Wed, May 13, 2020 at 07:33:56PM +0300, Fotis Xenakis wrote:
> Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
> descriptor for the file to mmap() to QEMU (specifically, it passes the
> file handle as requested from the guest).
>
> This fixes it, using lo_fi_fd() to map the file handle to the right file
> descriptor on the host.
>
Looks good, guest has been setting fi as NULL though.
Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>
> Signed-off-by: Fotis Xenakis <foxen@windowslive.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 e09d557d74..44bb24eb5e 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2709,7 +2709,7 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
> msg.flags[0] = vhu_flags;
>
> if (fi) {
> - fd = fi->fh;
> + fd = lo_fi_fd(req, fi);
> } else {
> res = asprintf(&buf, "%i", lo_fd(req, ino));
> if (res == -1) {
> --
> 2.26.2
>
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
2020-05-13 16:33 [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping Fotis Xenakis
2020-05-15 3:48 ` Liu Bo
@ 2020-05-15 12:52 ` Vivek Goyal
2020-05-18 10:45 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 7+ messages in thread
From: Vivek Goyal @ 2020-05-15 12:52 UTC (permalink / raw)
To: Fotis Xenakis; +Cc: virtio-fs
On Wed, May 13, 2020 at 07:33:56PM +0300, Fotis Xenakis wrote:
> Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
> descriptor for the file to mmap() to QEMU (specifically, it passes the
> file handle as requested from the guest).
>
> This fixes it, using lo_fi_fd() to map the file handle to the right file
> descriptor on the host.
>
> Signed-off-by: Fotis Xenakis <foxen@windowslive.com>
Acked-by: Vivek Goyal <vgoyal@redhat.com>
Vivek
> ---
> 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 e09d557d74..44bb24eb5e 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2709,7 +2709,7 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
> msg.flags[0] = vhu_flags;
>
> if (fi) {
> - fd = fi->fh;
> + fd = lo_fi_fd(req, fi);
> } else {
> res = asprintf(&buf, "%i", lo_fd(req, ino));
> if (res == -1) {
> --
> 2.26.2
>
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
2020-05-15 3:48 ` Liu Bo
@ 2020-05-15 16:50 ` Fotis Xenakis
2020-05-18 12:37 ` Vivek Goyal
0 siblings, 1 reply; 7+ messages in thread
From: Fotis Xenakis @ 2020-05-15 16:50 UTC (permalink / raw)
To: bo.liu; +Cc: virtio-fs
[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]
From: Liu Bo <bo.liu@linux.alibaba.com>
Sent: Friday, May 15, 2020 06:48
To: Fotis Xenakis <foxen@windowslive.com>
Cc: virtio-fs@redhat.com <virtio-fs@redhat.com>
Subject: Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
On Wed, May 13, 2020 at 07:33:56PM +0300, Fotis Xenakis wrote:
> Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
> descriptor for the file to mmap() to QEMU (specifically, it passes the
> file handle as requested from the guest).
>
> This fixes it, using lo_fi_fd() to map the file handle to the right file
> descriptor on the host.
>
Looks good, guest has been setting fi as NULL though.
As far as I can see, this function is only called from [1], which passes fi as NULL only when the file handle passed from the guest is (uint64_t)-1. In my testing this was never the case though.
Reference:
[1] https://gitlab.com/virtio-fs/qemu/-/blob/21336c0f3d05a97f5c409bbc894c19d87259655c/tools/virtiofsd/fuse_lowlevel.c#L1930
Reviewed-by: Liu Bo <bo.liu@linux.alibaba.com>
> Signed-off-by: Fotis Xenakis <foxen@windowslive.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 e09d557d74..44bb24eb5e 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2709,7 +2709,7 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
> msg.flags[0] = vhu_flags;
>
> if (fi) {
> - fd = fi->fh;
> + fd = lo_fi_fd(req, fi);
> } else {
> res = asprintf(&buf, "%i", lo_fd(req, ino));
> if (res == -1) {
> --
> 2.26.2
>
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
[-- Attachment #2: Type: text/html, Size: 4129 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
2020-05-13 16:33 [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping Fotis Xenakis
2020-05-15 3:48 ` Liu Bo
2020-05-15 12:52 ` Vivek Goyal
@ 2020-05-18 10:45 ` Dr. David Alan Gilbert
2 siblings, 0 replies; 7+ messages in thread
From: Dr. David Alan Gilbert @ 2020-05-18 10:45 UTC (permalink / raw)
To: Fotis Xenakis; +Cc: virtio-fs
* Fotis Xenakis (foxen@windowslive.com) wrote:
> Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
> descriptor for the file to mmap() to QEMU (specifically, it passes the
> file handle as requested from the guest).
>
> This fixes it, using lo_fi_fd() to map the file handle to the right file
> descriptor on the host.
>
> Signed-off-by: Fotis Xenakis <foxen@windowslive.com>
Thanks! I've merged this into:
'DAX: virtiofsd: Make setupmapping work only with inode'
This was a fixup/merging problem - the fd indirection/hiding was done
after the setupmapping; but then the fd indirection got merged first and
the setupmapping put back on top.
Dave
> ---
> 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 e09d557d74..44bb24eb5e 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -2709,7 +2709,7 @@ static void lo_setupmapping(fuse_req_t req, fuse_ino_t ino, uint64_t foffset,
> msg.flags[0] = vhu_flags;
>
> if (fi) {
> - fd = fi->fh;
> + fd = lo_fi_fd(req, fi);
> } else {
> res = asprintf(&buf, "%i", lo_fd(req, ino));
> if (res == -1) {
> --
> 2.26.2
>
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
2020-05-15 16:50 ` Fotis Xenakis
@ 2020-05-18 12:37 ` Vivek Goyal
2020-05-18 21:17 ` Fotis Xenakis
0 siblings, 1 reply; 7+ messages in thread
From: Vivek Goyal @ 2020-05-18 12:37 UTC (permalink / raw)
To: Fotis Xenakis; +Cc: virtio-fs
On Fri, May 15, 2020 at 04:50:31PM +0000, Fotis Xenakis wrote:
> From: Liu Bo <bo.liu@linux.alibaba.com>
> Sent: Friday, May 15, 2020 06:48
> To: Fotis Xenakis <foxen@windowslive.com>
> Cc: virtio-fs@redhat.com <virtio-fs@redhat.com>
> Subject: Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
>
> On Wed, May 13, 2020 at 07:33:56PM +0300, Fotis Xenakis wrote:
> > Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
> > descriptor for the file to mmap() to QEMU (specifically, it passes the
> > file handle as requested from the guest).
> >
> > This fixes it, using lo_fi_fd() to map the file handle to the right file
> > descriptor on the host.
> >
>
> Looks good, guest has been setting fi as NULL though.
> As far as I can see, this function is only called from [1], which passes fi as NULL only when the file handle passed from the guest is (uint64_t)-1. In my testing this was never the case though.
Hi Fotis,
Do you know what's different about your setup. This has been working for
me so clearly I am seeing fi as NULL.
Vivek
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
2020-05-18 12:37 ` Vivek Goyal
@ 2020-05-18 21:17 ` Fotis Xenakis
0 siblings, 0 replies; 7+ messages in thread
From: Fotis Xenakis @ 2020-05-18 21:17 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs
[-- Attachment #1: Type: text/plain, Size: 2424 bytes --]
From: Vivek Goyal <vgoyal@redhat.com>
Sent: Monday, May 18, 2020 15:37
To: Fotis Xenakis <foxen@windowslive.com>
Cc: bo.liu@linux.alibaba.com <bo.liu@linux.alibaba.com>; virtio-fs@redhat.com <virtio-fs@redhat.com>
Subject: Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
On Fri, May 15, 2020 at 04:50:31PM +0000, Fotis Xenakis wrote:
> From: Liu Bo <bo.liu@linux.alibaba.com>
> Sent: Friday, May 15, 2020 06:48
> To: Fotis Xenakis <foxen@windowslive.com>
> Cc: virtio-fs@redhat.com <virtio-fs@redhat.com>
> Subject: Re: [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping
>
> On Wed, May 13, 2020 at 07:33:56PM +0300, Fotis Xenakis wrote:
> > Currently, during FUSE_SETUPMAPPING, virtiofsd passes the wrong file
> > descriptor for the file to mmap() to QEMU (specifically, it passes the
> > file handle as requested from the guest).
> >
> > This fixes it, using lo_fi_fd() to map the file handle to the right file
> > descriptor on the host.
> >
>
> Looks good, guest has been setting fi as NULL though.
> As far as I can see, this function is only called from [1], which passes fi as NULL only when the file handle passed from the guest is (uint64_t)-1. In my testing this was never the case though.
Hi Fotis,
Do you know what's different about your setup. This has been working for
me so clearly I am seeing fi as NULL.
Hello,
This was on my mind as well: if others were affected by this bug, it wouldn't have gone unnoticed. Yet, I can't say I know what's different in my setup.
Unfortunately, I have a less-than-basic understanding of virtiofsd, but as far as I can see:
- virtiofsd differentiates on the file handler passed by the guest [1]. In my case fh != (uint64_t)-1 (which makes sense tbh). fh is whatever FUSE_OPEN returned.
- Guest-side, the open() flags are passed through to FUSE_OPEN [2]. In my case that's O_RDONLY.
- The only flag set for FUSE_INIT is FUSE_MAP_ALIGNMENT [3].
- For what it's worth, virtiofsd runs with "-o cache=always".
References:
[1] https://gitlab.com/virtio-fs/qemu/-/blob/21336c0f3d05a97f5c409bbc894c19d87259655c/tools/virtiofsd/fuse_lowlevel.c#L1910
[2] https://github.com/foxeng/osv/blob/46255086015a4c801baaf61d431e3aa7eda9e64b/fs/virtiofs/virtiofs_vnops.cc#L111
[3] https://github.com/foxeng/osv/blob/46255086015a4c801baaf61d431e3aa7eda9e64b/fs/virtiofs/virtiofs_vfsops.cc#L91
Vivek
[-- Attachment #2: Type: text/html, Size: 6731 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-18 21:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 16:33 [Virtio-fs] [PATCH] Fix file descriptor passed for setupmapping Fotis Xenakis
2020-05-15 3:48 ` Liu Bo
2020-05-15 16:50 ` Fotis Xenakis
2020-05-18 12:37 ` Vivek Goyal
2020-05-18 21:17 ` Fotis Xenakis
2020-05-15 12:52 ` Vivek Goyal
2020-05-18 10:45 ` Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.