Patchwork [PATCHv2,4/4] Warning messages on net devices hotplug

login
register
mail settings
Submitter Eduardo Otubo
Date Oct. 23, 2012, 5:55 a.m.
Message ID <1350971732-16621-4-git-send-email-otubo@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/193360/
State New
Headers show

Comments

Eduardo Otubo - Oct. 23, 2012, 5:55 a.m.
With the inclusion of the new "double whitelist" seccomp filter, Qemu
won't be able to execve() in runtime, thus, no hotplug net devices
allowed.

v2: * Error messages moved to the backend function, net_init_tap(), recommended
      by Paolo Bonzini
    * Documentation added to QMP and HMP commands, and also to the Qemu options.

Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
---
 hmp-commands.hx | 4 ++--
 net.c           | 1 +
 net/tap.c       | 5 +++++
 qemu-options.hx | 3 ++-
 qmp-commands.hx | 3 ++-
 5 files changed, 12 insertions(+), 4 deletions(-)
Corey Bryant - Oct. 23, 2012, 3:59 p.m.
On 10/23/2012 01:55 AM, Eduardo Otubo wrote:
> With the inclusion of the new "double whitelist" seccomp filter, Qemu
> won't be able to execve() in runtime, thus, no hotplug net devices
> allowed.
>
> v2: * Error messages moved to the backend function, net_init_tap(), recommended
>        by Paolo Bonzini
>      * Documentation added to QMP and HMP commands, and also to the Qemu options.
>
> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
> ---
>   hmp-commands.hx | 4 ++--
>   net.c           | 1 +
>   net/tap.c       | 5 +++++
>   qemu-options.hx | 3 ++-
>   qmp-commands.hx | 3 ++-
>   5 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index e0b537d..3e28501 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1068,7 +1068,7 @@ ETEXI
>           .name       = "host_net_add",
>           .args_type  = "device:s,opts:s?",
>           .params     = "tap|user|socket|vde|dump [options]",
> -        .help       = "add host VLAN client",
> +        .help       = "add host VLAN client, feature disabled when -sandbox is in use",

Maybe the "feature disabled.." part should be in parenthesis?  There's 
also another section in hmp-commands.hx a few lines below this code 
where you can add the same note.

>           .mhandler.cmd = net_host_device_add,
>       },
>
> @@ -1096,7 +1096,7 @@ ETEXI
>           .name       = "netdev_add",
>           .args_type  = "netdev:O",
>           .params     = "[user|tap|socket],id=str[,prop=value][,...]",
> -        .help       = "add host network device",
> +        .help       = "add host network device, feature disabled when -sandbox is in use",

Same comments as above.

>           .mhandler.cmd = hmp_netdev_add,
>       },
>
> diff --git a/net.c b/net.c
> index ae4bc0d..02188f0 100644
> --- a/net.c
> +++ b/net.c
> @@ -765,6 +765,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>       qemu_opt_set(opts, "type", device);
>
>       net_client_init(opts, 0, &local_err);
> +

You can get rid of this change.

>       if (error_is_set(&local_err)) {
>           qerror_report_err(local_err);
>           error_free(local_err);
> diff --git a/net/tap.c b/net/tap.c
> index df89caa..dd8c79b 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -590,6 +590,11 @@ static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
>   int net_init_tap(const NetClientOptions *opts, const char *name,
>                    NetClientState *peer)
>   {
> +#ifdef CONFIG_SECCOMP
> +    error_report("Cannot hotplug TAP device when -sandbox is in effect");
> +    return -1;
> +#endif
> +

It seems like it would make more sense to put this code after the local 
variables are defined.  But more importantly, does this prevent adding a 
tap device on the command line?  If so, that's not good.  Also I don't 
think you are preventing a "netdev_add bridge" any more in v2, which 
also calls execve().

>       const NetdevTapOptions *tap;
>
>       int fd, vnet_hdr = 0;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 7d97f96..02afba3 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2767,7 +2767,8 @@ STEXI
>   @item -sandbox
>   @findex -sandbox
>   Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will
> -disable it.  The default is 'off'.
> +disable it.  The default is 'on'. Note that when using the '-sandbox on' option the hot plug
> +of new devices will be disabled.

Only network devices are prevented, right?

Also, as I mentioned before, can you limit this to the subset of options 
that cause execve() to be issued?  For example, can we allow libvirt to 
pass an fd for hotplugging a network device (e.g. netdev_add tap,fd=23)? 
  I don't know for sure but I'm guessing libvirt does that.

Also I think it would be worth-while to update the qemu-kvm --help 
output with a similar note too.

>   ETEXI
>
>   DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 2f8477e..cccb8f1 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -757,7 +757,8 @@ Example:
>
>   Note: The supported device options are the same ones supported by the '-net'
>         command-line argument, which are listed in the '-help' output or QEMU's
> -      manual
> +      manual. Also note that the hot plug is disabled when -sandbox is in
> +      effect

Not all hotplug abilities are disabled.  Just network devices.  This is 
missing a period too.

>
>   EQMP
>
Eric Blake - Oct. 23, 2012, 4:39 p.m.
On 10/23/2012 09:59 AM, Corey Bryant wrote:

> Only network devices are prevented, right?
> 
> Also, as I mentioned before, can you limit this to the subset of options
> that cause execve() to be issued?  For example, can we allow libvirt to
> pass an fd for hotplugging a network device (e.g. netdev_add tap,fd=23)?
>  I don't know for sure but I'm guessing libvirt does that.

Correct, libvirt prefers passing network devices pre-opened via fds,
rather than having qemu exec scripts.

>> +      manual. Also note that the hot plug is disabled when -sandbox
>> is in
>> +      effect
> 
> Not all hotplug abilities are disabled.  Just network devices.  This is
> missing a period too.

And not all network hotplug, just hotplug that requires use of exec
(again, fd passing bypasses the need for exec).

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e0b537d..3e28501 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1068,7 +1068,7 @@  ETEXI
         .name       = "host_net_add",
         .args_type  = "device:s,opts:s?",
         .params     = "tap|user|socket|vde|dump [options]",
-        .help       = "add host VLAN client",
+        .help       = "add host VLAN client, feature disabled when -sandbox is in use",
         .mhandler.cmd = net_host_device_add,
     },
 
@@ -1096,7 +1096,7 @@  ETEXI
         .name       = "netdev_add",
         .args_type  = "netdev:O",
         .params     = "[user|tap|socket],id=str[,prop=value][,...]",
-        .help       = "add host network device",
+        .help       = "add host network device, feature disabled when -sandbox is in use",
         .mhandler.cmd = hmp_netdev_add,
     },
 
diff --git a/net.c b/net.c
index ae4bc0d..02188f0 100644
--- a/net.c
+++ b/net.c
@@ -765,6 +765,7 @@  void net_host_device_add(Monitor *mon, const QDict *qdict)
     qemu_opt_set(opts, "type", device);
 
     net_client_init(opts, 0, &local_err);
+
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
         error_free(local_err);
diff --git a/net/tap.c b/net/tap.c
index df89caa..dd8c79b 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -590,6 +590,11 @@  static int net_tap_init(const NetdevTapOptions *tap, int *vnet_hdr,
 int net_init_tap(const NetClientOptions *opts, const char *name,
                  NetClientState *peer)
 {
+#ifdef CONFIG_SECCOMP
+    error_report("Cannot hotplug TAP device when -sandbox is in effect");
+    return -1;
+#endif
+
     const NetdevTapOptions *tap;
 
     int fd, vnet_hdr = 0;
diff --git a/qemu-options.hx b/qemu-options.hx
index 7d97f96..02afba3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2767,7 +2767,8 @@  STEXI
 @item -sandbox
 @findex -sandbox
 Enable Seccomp mode 2 system call filter. 'on' will enable syscall filtering and 'off' will
-disable it.  The default is 'off'.
+disable it.  The default is 'on'. Note that when using the '-sandbox on' option the hot plug
+of new devices will be disabled.
 ETEXI
 
 DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..cccb8f1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -757,7 +757,8 @@  Example:
 
 Note: The supported device options are the same ones supported by the '-net'
       command-line argument, which are listed in the '-help' output or QEMU's
-      manual
+      manual. Also note that the hot plug is disabled when -sandbox is in
+      effect
 
 EQMP