* [PATCH] virtiofsd: Disable killpriv_v2 by default
@ 2022-07-29 13:17 Vivek Goyal
2022-08-02 10:33 ` Dr. David Alan Gilbert
2022-08-02 13:57 ` Dr. David Alan Gilbert
0 siblings, 2 replies; 3+ messages in thread
From: Vivek Goyal @ 2022-07-29 13:17 UTC (permalink / raw)
To: virtio-fs-list, Dr. David Alan Gilbert, qemu-devel; +Cc: Miklos Szeredi
We are having bunch of issues with killpriv_v2 enabled by default. First
of all it relies on clearing suid/sgid bits as needed by dropping
capability CAP_FSETID. This does not work for remote filesystems like
NFS (and possibly others).
Secondly, we are noticing other issues related to clearing of SGID
which leads to failures for xfstests generic/355 and generic/193.
Thirdly, there are other issues w.r.t caching of metadata (suid/sgid)
bits in fuse client with killpriv_v2 enabled. Guest can cache that
data for sometime even if cleared on server.
Second and Third issue are fixable. Just that it might take a little
while to get it fixed in kernel. First one will probably not see
any movement for a long time.
Given these issues, killpriv_v2 does not seem to be a good candidate
for enabling by default. We have already disabled it by default in
rust version of virtiofsd.
Hence this patch disabled killpriv_v2 by default. User can choose to
enable it by passing option "-o killpriv_v2".
Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
===================================================================
--- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2022-07-29 08:19:05.925119947 -0400
+++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c 2022-07-29 08:27:08.048049096 -0400
@@ -767,19 +767,10 @@ static void lo_init(void *userdata, stru
fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
lo->killpriv_v2 = 1;
- } else if (lo->user_killpriv_v2 == -1 &&
- conn->capable & FUSE_CAP_HANDLE_KILLPRIV_V2) {
- /*
- * User did not specify a value for killpriv_v2. By default enable it
- * if connection offers this capability
- */
- fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
- conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
- lo->killpriv_v2 = 1;
} else {
/*
- * Either user specified to disable killpriv_v2, or connection does
- * not offer this capability. Disable killpriv_v2 in both the cases
+ * Either user specified to disable killpriv_v2, or did not
+ * specify anything. Disable killpriv_v2 in both the cases.
*/
fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling killpriv_v2\n");
conn->want &= ~FUSE_CAP_HANDLE_KILLPRIV_V2;
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] virtiofsd: Disable killpriv_v2 by default
2022-07-29 13:17 [PATCH] virtiofsd: Disable killpriv_v2 by default Vivek Goyal
@ 2022-08-02 10:33 ` Dr. David Alan Gilbert
2022-08-02 13:57 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 3+ messages in thread
From: Dr. David Alan Gilbert @ 2022-08-02 10:33 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs-list, qemu-devel, Miklos Szeredi
* Vivek Goyal (vgoyal@redhat.com) wrote:
> We are having bunch of issues with killpriv_v2 enabled by default. First
> of all it relies on clearing suid/sgid bits as needed by dropping
> capability CAP_FSETID. This does not work for remote filesystems like
> NFS (and possibly others).
>
> Secondly, we are noticing other issues related to clearing of SGID
> which leads to failures for xfstests generic/355 and generic/193.
>
> Thirdly, there are other issues w.r.t caching of metadata (suid/sgid)
> bits in fuse client with killpriv_v2 enabled. Guest can cache that
> data for sometime even if cleared on server.
>
> Second and Third issue are fixable. Just that it might take a little
> while to get it fixed in kernel. First one will probably not see
> any movement for a long time.
>
> Given these issues, killpriv_v2 does not seem to be a good candidate
> for enabling by default. We have already disabled it by default in
> rust version of virtiofsd.
>
> Hence this patch disabled killpriv_v2 by default. User can choose to
> enable it by passing option "-o killpriv_v2".
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
OK, yes I see the corresponding 9b03f65d commit in the Rust version.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> tools/virtiofsd/passthrough_ll.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===================================================================
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2022-07-29 08:19:05.925119947 -0400
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c 2022-07-29 08:27:08.048049096 -0400
> @@ -767,19 +767,10 @@ static void lo_init(void *userdata, stru
> fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
> conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
> lo->killpriv_v2 = 1;
> - } else if (lo->user_killpriv_v2 == -1 &&
> - conn->capable & FUSE_CAP_HANDLE_KILLPRIV_V2) {
> - /*
> - * User did not specify a value for killpriv_v2. By default enable it
> - * if connection offers this capability
> - */
> - fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
> - conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
> - lo->killpriv_v2 = 1;
> } else {
> /*
> - * Either user specified to disable killpriv_v2, or connection does
> - * not offer this capability. Disable killpriv_v2 in both the cases
> + * Either user specified to disable killpriv_v2, or did not
> + * specify anything. Disable killpriv_v2 in both the cases.
> */
> fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling killpriv_v2\n");
> conn->want &= ~FUSE_CAP_HANDLE_KILLPRIV_V2;
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] virtiofsd: Disable killpriv_v2 by default
2022-07-29 13:17 [PATCH] virtiofsd: Disable killpriv_v2 by default Vivek Goyal
2022-08-02 10:33 ` Dr. David Alan Gilbert
@ 2022-08-02 13:57 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 3+ messages in thread
From: Dr. David Alan Gilbert @ 2022-08-02 13:57 UTC (permalink / raw)
To: Vivek Goyal; +Cc: virtio-fs-list, qemu-devel, Miklos Szeredi
* Vivek Goyal (vgoyal@redhat.com) wrote:
> We are having bunch of issues with killpriv_v2 enabled by default. First
> of all it relies on clearing suid/sgid bits as needed by dropping
> capability CAP_FSETID. This does not work for remote filesystems like
> NFS (and possibly others).
>
> Secondly, we are noticing other issues related to clearing of SGID
> which leads to failures for xfstests generic/355 and generic/193.
>
> Thirdly, there are other issues w.r.t caching of metadata (suid/sgid)
> bits in fuse client with killpriv_v2 enabled. Guest can cache that
> data for sometime even if cleared on server.
>
> Second and Third issue are fixable. Just that it might take a little
> while to get it fixed in kernel. First one will probably not see
> any movement for a long time.
>
> Given these issues, killpriv_v2 does not seem to be a good candidate
> for enabling by default. We have already disabled it by default in
> rust version of virtiofsd.
>
> Hence this patch disabled killpriv_v2 by default. User can choose to
> enable it by passing option "-o killpriv_v2".
>
> Signed-off-by: Vivek Goyal <vgoyal@redhat.com>
Queued
> ---
> tools/virtiofsd/passthrough_ll.c | 13 ++-----------
> 1 file changed, 2 insertions(+), 11 deletions(-)
>
> Index: rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c
> ===================================================================
> --- rhvgoyal-qemu.orig/tools/virtiofsd/passthrough_ll.c 2022-07-29 08:19:05.925119947 -0400
> +++ rhvgoyal-qemu/tools/virtiofsd/passthrough_ll.c 2022-07-29 08:27:08.048049096 -0400
> @@ -767,19 +767,10 @@ static void lo_init(void *userdata, stru
> fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
> conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
> lo->killpriv_v2 = 1;
> - } else if (lo->user_killpriv_v2 == -1 &&
> - conn->capable & FUSE_CAP_HANDLE_KILLPRIV_V2) {
> - /*
> - * User did not specify a value for killpriv_v2. By default enable it
> - * if connection offers this capability
> - */
> - fuse_log(FUSE_LOG_DEBUG, "lo_init: enabling killpriv_v2\n");
> - conn->want |= FUSE_CAP_HANDLE_KILLPRIV_V2;
> - lo->killpriv_v2 = 1;
> } else {
> /*
> - * Either user specified to disable killpriv_v2, or connection does
> - * not offer this capability. Disable killpriv_v2 in both the cases
> + * Either user specified to disable killpriv_v2, or did not
> + * specify anything. Disable killpriv_v2 in both the cases.
> */
> fuse_log(FUSE_LOG_DEBUG, "lo_init: disabling killpriv_v2\n");
> conn->want &= ~FUSE_CAP_HANDLE_KILLPRIV_V2;
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-02 13:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-29 13:17 [PATCH] virtiofsd: Disable killpriv_v2 by default Vivek Goyal
2022-08-02 10:33 ` Dr. David Alan Gilbert
2022-08-02 13:57 ` Dr. David Alan Gilbert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).