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 05:48:13
Also in:
lkml, virtualization
On Mon, Mar 3, 2025 at 5:12 PM Stefano Garzarella [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; } +nit: this empyt line is not needed
sure , will fix this Thanks Cindy
quoted
quoted
+#else + if (ioctl == VHOST_FORK_FROM_OWNER) { + /* When CONFIG_VHOST_ENABLE_FORK_OWNER_IOCTL is 'n', return error */ + r = -ENOTTY; + goto done; + } +#endif + /* You must be the owner to do anything else */ r = vhost_dev_check_owner(d); if (r) -- 2.45.0Do 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'm not sure about this honestly, the user space has no way to figure out the default value and still has to do the IOCTL. So IMHO better to have a default value that is independent of the kernel configuration and consistent with the current behavior. Thanks, Stefanoquoted
Other patches look good to me. Thanksquoted