Thread (22 messages) 22 messages, 5 authors, 2025-03-28

Re: [PATCH v7 8/8] vhost: Add a KConfig knob to enable IOCTL VHOST_FORK_FROM_OWNER

From: Cindy Lu <hidden>
Date: 2025-03-28 08:24:57
Also in: lkml, virtualization

On Tue, Mar 4, 2025 at 1:33 AM Michael S. Tsirkin [off-list ref] wrote:
On Mon, Mar 03, 2025 at 01:52:06PM +0800, Jason Wang wrote:
quoted
On Sun, Mar 2, 2025 at 10:34 PM Cindy Lu [off-list ref] wrote:
quoted
Introduce a new config knob `CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL`,
to control the availability of the `VHOST_FORK_FROM_OWNER` ioctl.
When CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL is set to n, the ioctl
is disabled, and any attempt to use it will result in failure.

Signed-off-by: Cindy Lu <redacted>
---
 drivers/vhost/Kconfig | 15 +++++++++++++++
 drivers/vhost/vhost.c | 11 +++++++++++
 2 files changed, 26 insertions(+)
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index b455d9ab6f3d..e5b9dcbf31b6 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -95,3 +95,18 @@ config VHOST_CROSS_ENDIAN_LEGACY
          If unsure, say "N".

 endif
+
+config VHOST_ENABLE_FORK_OWNER_IOCTL
+       bool "Enable IOCTL VHOST_FORK_FROM_OWNER"
+       default n
+       help
+         This option enables the IOCTL VHOST_FORK_FROM_OWNER, which allows
+         userspace applications to modify the thread mode for vhost devices.
+
+          By default, `CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL` is set to `n`,
+          meaning the ioctl is disabled and any operation using this ioctl
+          will fail.
+          When the configuration is enabled (y), the ioctl becomes
+          available, allowing users to set the mode if needed.
+
+         If unsure, say "N".
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index fb0c7fb43f78..09e5e44dc516 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2294,6 +2294,8 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
                r = vhost_dev_set_owner(d);
                goto done;
        }
+
+#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL
        if (ioctl == VHOST_FORK_FROM_OWNER) {
                u8 inherit_owner;
                /*inherit_owner can only be modified before owner is set*/
@@ -2313,6 +2315,15 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
                r = 0;
                goto done;
        }
+
+#else
+       if (ioctl == VHOST_FORK_FROM_OWNER) {
+               /* When CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL is 'n', return error */
+               r = -ENOTTY;
+               goto done;
+       }
why do we need this? won't it fail as any other unsupported ioctl?
Sure, will remove  this
thanks
cindy
quoted
quoted
+#endif
+
        /* You must be the owner to do anything else */
        r = vhost_dev_check_owner(d);
        if (r)
--
2.45.0
Do we need to change the default value of the inhert_owner? For example:

#ifdef CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL
inherit_owner = false;
#else
inherit_onwer = true;
#endif

?
I feel it is best to keep the default consistent.
All the kconfig should do, is block the ioctl.

quoted
Other patches look good to me.

Thanks
quoted
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help