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

login
register
mail settings
Submitter Eduardo Otubo
Date Oct. 17, 2012, 1:15 p.m.
Message ID <1350479712-15082-4-git-send-email-otubo@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/192039/
State New
Headers show

Comments

Eduardo Otubo - Oct. 17, 2012, 1:15 p.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.

Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
---
 hmp.c |  6 ++++++
 net.c | 13 +++++++++++++
 2 files changed, 19 insertions(+)
Corey Bryant - Oct. 18, 2012, 2:59 p.m.
On 10/17/2012 09:15 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.
>
> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
> ---
>   hmp.c |  6 ++++++
>   net.c | 13 +++++++++++++
>   2 files changed, 19 insertions(+)
>
> diff --git a/hmp.c b/hmp.c
> index 70bdec2..f258338 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1091,6 +1091,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
>       Error *err = NULL;
>       QemuOpts *opts;
>
> +#ifdef CONFIG_SECCOMP
> +    error_set(&err, ERROR_CLASS_GENERIC_ERROR,
> +            "Cannot hotplug TAP device when -sandbox is in effect");
> +    goto out;
> +#endif
> +
>       opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);
>       if (error_is_set(&err)) {
>           goto out;
> diff --git a/net.c b/net.c
> index ae4bc0d..a652ee9 100644
> --- a/net.c
> +++ b/net.c
> @@ -752,6 +752,12 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>       Error *local_err = NULL;
>       QemuOpts *opts;
>
> +#ifdef CONFIG_SECCOMP
> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
> +            "Cannot hotplug TAP device when -sandbox is in effect");
> +    goto out;
> +#endif
> +
>       if (!net_host_check_device(device)) {
>           monitor_printf(mon, "invalid host network device %s\n", device);
>           return;
> @@ -765,6 +771,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>       qemu_opt_set(opts, "type", device);
>
>       net_client_init(opts, 0, &local_err);
> +out:
>       if (error_is_set(&local_err)) {
>           qerror_report_err(local_err);
>           error_free(local_err);
> @@ -800,6 +807,12 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
>       QemuOptsList *opts_list;
>       QemuOpts *opts;
>
> +#ifdef CONFIG_SECCOMP
> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
> +            "Cannot hotplug TAP device when -sandbox is in effect");
> +    goto exit_err;
> +#endif
> +
>       opts_list = qemu_find_opts_err("netdev", &local_err);
>       if (error_is_set(&local_err)) {
>           goto exit_err;
>

I think you need to either remove "TAP" from these messages, or limit 
this new code to tap and bridge since those are the backends that call 
execve().

Also, this should be documented somewhere so that users can find out 
about this behavior before attempting to hotplug a network device. 
Perhaps this could be documented on the man page for -sandbox and notes 
could be added to the HMP/QMP commands.
Paolo Bonzini - Oct. 18, 2012, 3:15 p.m.
Il 17/10/2012 15:15, Eduardo Otubo ha scritto:
> 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.
> 
> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>

Please check this in net_init_tap instead.  When using libvirt, hotplug
is done with a completely different mechanism that involves
file-descriptor passing and does not require executing a helper.

Paolo

> ---
>  hmp.c |  6 ++++++
>  net.c | 13 +++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/hmp.c b/hmp.c
> index 70bdec2..f258338 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1091,6 +1091,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
>      Error *err = NULL;
>      QemuOpts *opts;
>  
> +#ifdef CONFIG_SECCOMP
> +    error_set(&err, ERROR_CLASS_GENERIC_ERROR,
> +            "Cannot hotplug TAP device when -sandbox is in effect");
> +    goto out;
> +#endif
> +
>      opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);
>      if (error_is_set(&err)) {
>          goto out;
> diff --git a/net.c b/net.c
> index ae4bc0d..a652ee9 100644
> --- a/net.c
> +++ b/net.c
> @@ -752,6 +752,12 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>      Error *local_err = NULL;
>      QemuOpts *opts;
>  
> +#ifdef CONFIG_SECCOMP
> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
> +            "Cannot hotplug TAP device when -sandbox is in effect");
> +    goto out;
> +#endif
> +
>      if (!net_host_check_device(device)) {
>          monitor_printf(mon, "invalid host network device %s\n", device);
>          return;
> @@ -765,6 +771,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>      qemu_opt_set(opts, "type", device);
>  
>      net_client_init(opts, 0, &local_err);
> +out:
>      if (error_is_set(&local_err)) {
>          qerror_report_err(local_err);
>          error_free(local_err);
> @@ -800,6 +807,12 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
>      QemuOptsList *opts_list;
>      QemuOpts *opts;
>  
> +#ifdef CONFIG_SECCOMP
> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
> +            "Cannot hotplug TAP device when -sandbox is in effect");
> +    goto exit_err;
> +#endif
> +
>      opts_list = qemu_find_opts_err("netdev", &local_err);
>      if (error_is_set(&local_err)) {
>          goto exit_err;
>
Corey Bryant - Oct. 24, 2012, 2:18 p.m.
On 10/18/2012 11:15 AM, Paolo Bonzini wrote:
> Il 17/10/2012 15:15, Eduardo Otubo ha scritto:
>> 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.
>>
>> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
>
> Please check this in net_init_tap instead.  When using libvirt, hotplug
> is done with a completely different mechanism that involves
> file-descriptor passing and does not require executing a helper.
>
> Paolo
>

Are you sure net_init_tap() is the right place for this check?  We only 
want to prevent execve() after main_loop() is entered.  In other words 
we want to allow execve() caused by a command line option (e.g. -net 
tap) but we want to prevent execve() when it is the result of a monitor 
command (e.g. netdev_add tap).

>> ---
>>   hmp.c |  6 ++++++
>>   net.c | 13 +++++++++++++
>>   2 files changed, 19 insertions(+)
>>
>> diff --git a/hmp.c b/hmp.c
>> index 70bdec2..f258338 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -1091,6 +1091,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
>>       Error *err = NULL;
>>       QemuOpts *opts;
>>
>> +#ifdef CONFIG_SECCOMP
>> +    error_set(&err, ERROR_CLASS_GENERIC_ERROR,
>> +            "Cannot hotplug TAP device when -sandbox is in effect");
>> +    goto out;
>> +#endif
>> +
>>       opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);
>>       if (error_is_set(&err)) {
>>           goto out;
>> diff --git a/net.c b/net.c
>> index ae4bc0d..a652ee9 100644
>> --- a/net.c
>> +++ b/net.c
>> @@ -752,6 +752,12 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>>       Error *local_err = NULL;
>>       QemuOpts *opts;
>>
>> +#ifdef CONFIG_SECCOMP
>> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
>> +            "Cannot hotplug TAP device when -sandbox is in effect");
>> +    goto out;
>> +#endif
>> +
>>       if (!net_host_check_device(device)) {
>>           monitor_printf(mon, "invalid host network device %s\n", device);
>>           return;
>> @@ -765,6 +771,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict)
>>       qemu_opt_set(opts, "type", device);
>>
>>       net_client_init(opts, 0, &local_err);
>> +out:
>>       if (error_is_set(&local_err)) {
>>           qerror_report_err(local_err);
>>           error_free(local_err);
>> @@ -800,6 +807,12 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
>>       QemuOptsList *opts_list;
>>       QemuOpts *opts;
>>
>> +#ifdef CONFIG_SECCOMP
>> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
>> +            "Cannot hotplug TAP device when -sandbox is in effect");
>> +    goto exit_err;
>> +#endif
>> +
>>       opts_list = qemu_find_opts_err("netdev", &local_err);
>>       if (error_is_set(&local_err)) {
>>           goto exit_err;
>>
>
>
>
>
Corey Bryant - Oct. 24, 2012, 2:34 p.m.
On 10/24/2012 10:18 AM, Corey Bryant wrote:
>
>
> On 10/18/2012 11:15 AM, Paolo Bonzini wrote:
>> Il 17/10/2012 15:15, Eduardo Otubo ha scritto:
>>> 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.
>>>
>>> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
>>
>> Please check this in net_init_tap instead.  When using libvirt, hotplug
>> is done with a completely different mechanism that involves
>> file-descriptor passing and does not require executing a helper.
>>
>> Paolo
>>
>
> Are you sure net_init_tap() is the right place for this check?  We only
> want to prevent execve() after main_loop() is entered.  In other words
> we want to allow execve() caused by a command line option (e.g. -net
> tap) but we want to prevent execve() when it is the result of a monitor
> command (e.g. netdev_add tap).
>

Or perhaps we could put the check in net_init_tap() and only prevent the 
command when runstate != RUN_STATE_PRELAUNCH?

Note that we plan to only prevent the hotplug of net devices in the 
cases when execve() would be called.  So libvirt will still be able to 
pass an fd.

>>> ---
>>>   hmp.c |  6 ++++++
>>>   net.c | 13 +++++++++++++
>>>   2 files changed, 19 insertions(+)
>>>
>>> diff --git a/hmp.c b/hmp.c
>>> index 70bdec2..f258338 100644
>>> --- a/hmp.c
>>> +++ b/hmp.c
>>> @@ -1091,6 +1091,12 @@ void hmp_netdev_add(Monitor *mon, const QDict
>>> *qdict)
>>>       Error *err = NULL;
>>>       QemuOpts *opts;
>>>
>>> +#ifdef CONFIG_SECCOMP
>>> +    error_set(&err, ERROR_CLASS_GENERIC_ERROR,
>>> +            "Cannot hotplug TAP device when -sandbox is in effect");
>>> +    goto out;
>>> +#endif
>>> +
>>>       opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict,
>>> &err);
>>>       if (error_is_set(&err)) {
>>>           goto out;
>>> diff --git a/net.c b/net.c
>>> index ae4bc0d..a652ee9 100644
>>> --- a/net.c
>>> +++ b/net.c
>>> @@ -752,6 +752,12 @@ void net_host_device_add(Monitor *mon, const
>>> QDict *qdict)
>>>       Error *local_err = NULL;
>>>       QemuOpts *opts;
>>>
>>> +#ifdef CONFIG_SECCOMP
>>> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
>>> +            "Cannot hotplug TAP device when -sandbox is in effect");
>>> +    goto out;
>>> +#endif
>>> +
>>>       if (!net_host_check_device(device)) {
>>>           monitor_printf(mon, "invalid host network device %s\n",
>>> device);
>>>           return;
>>> @@ -765,6 +771,7 @@ void net_host_device_add(Monitor *mon, const
>>> QDict *qdict)
>>>       qemu_opt_set(opts, "type", device);
>>>
>>>       net_client_init(opts, 0, &local_err);
>>> +out:
>>>       if (error_is_set(&local_err)) {
>>>           qerror_report_err(local_err);
>>>           error_free(local_err);
>>> @@ -800,6 +807,12 @@ int qmp_netdev_add(Monitor *mon, const QDict
>>> *qdict, QObject **ret)
>>>       QemuOptsList *opts_list;
>>>       QemuOpts *opts;
>>>
>>> +#ifdef CONFIG_SECCOMP
>>> +    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
>>> +            "Cannot hotplug TAP device when -sandbox is in effect");
>>> +    goto exit_err;
>>> +#endif
>>> +
>>>       opts_list = qemu_find_opts_err("netdev", &local_err);
>>>       if (error_is_set(&local_err)) {
>>>           goto exit_err;
>>>
>>
>>
>>
>>
>
Paolo Bonzini - Oct. 24, 2012, 3:21 p.m.
Il 24/10/2012 16:18, Corey Bryant ha scritto:
> 
> 
> On 10/18/2012 11:15 AM, Paolo Bonzini wrote:
>> Il 17/10/2012 15:15, Eduardo Otubo ha scritto:
>>> 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.
>>>
>>> Signed-off-by: Eduardo Otubo <otubo@linux.vnet.ibm.com>
>>
>> Please check this in net_init_tap instead.  When using libvirt, hotplug
>> is done with a completely different mechanism that involves
>> file-descriptor passing and does not require executing a helper.
>>
>> Paolo
>>
> 
> Are you sure net_init_tap() is the right place for this check?

Yes, assuming there is a global that says whether the seccomp sandbox is
in effect.  Even something like "if (sandbox_active && !tap->has_fd)
error(...)" can be enough.

Paolo

Patch

diff --git a/hmp.c b/hmp.c
index 70bdec2..f258338 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1091,6 +1091,12 @@  void hmp_netdev_add(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
     QemuOpts *opts;
 
+#ifdef CONFIG_SECCOMP
+    error_set(&err, ERROR_CLASS_GENERIC_ERROR,
+            "Cannot hotplug TAP device when -sandbox is in effect");
+    goto out;
+#endif
+
     opts = qemu_opts_from_qdict(qemu_find_opts("netdev"), qdict, &err);
     if (error_is_set(&err)) {
         goto out;
diff --git a/net.c b/net.c
index ae4bc0d..a652ee9 100644
--- a/net.c
+++ b/net.c
@@ -752,6 +752,12 @@  void net_host_device_add(Monitor *mon, const QDict *qdict)
     Error *local_err = NULL;
     QemuOpts *opts;
 
+#ifdef CONFIG_SECCOMP
+    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
+            "Cannot hotplug TAP device when -sandbox is in effect");
+    goto out;
+#endif
+
     if (!net_host_check_device(device)) {
         monitor_printf(mon, "invalid host network device %s\n", device);
         return;
@@ -765,6 +771,7 @@  void net_host_device_add(Monitor *mon, const QDict *qdict)
     qemu_opt_set(opts, "type", device);
 
     net_client_init(opts, 0, &local_err);
+out:
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
         error_free(local_err);
@@ -800,6 +807,12 @@  int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret)
     QemuOptsList *opts_list;
     QemuOpts *opts;
 
+#ifdef CONFIG_SECCOMP
+    error_set(&local_err, ERROR_CLASS_GENERIC_ERROR,
+            "Cannot hotplug TAP device when -sandbox is in effect");
+    goto exit_err;
+#endif
+
     opts_list = qemu_find_opts_err("netdev", &local_err);
     if (error_is_set(&local_err)) {
         goto exit_err;