[Qemu-devel] [PATCH] net: avoid segfault in case netdev_del non-host network device

[Qemu-devel] [PATCH] net: avoid segfault in case netdev_del non-host network device

[Lei Li]

netdev_del assume that remove host network device.
However, when try to remove a non-host network device
by netdev_del, it will cause a segfault.

The reson is that qmp_netdev_del is not checking for
a NULL return for qemu_find_opts_err in case find_list
did not find the netdev group to delete.

Catch this and return an error.

(qemu) host_net_add user vlan=1,name=con.1,hostfwd=udp::4111-127.0.0.1:4333
(qemu) info network
hub 1
 \ con.1: type=user,net=10.0.2.0,restrict=off
hub 0
 \ user.0: type=user,net=10.0.2.0,restrict=off
 \ e1000.0: type=nic,model=e1000,macaddr=52:54:00:12:34:56
(qemu) netdev_del con.1
Segmentation fault (core dumped)

Signed-off-by: Lei Li <lilei@linux.vnet.ibm.com>
---
 net.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/net.c b/net.c
index ae4bc0d..cc52552 100644
--- a/net.c
+++ b/net.c
@@ -827,6 +827,7 @@ exit_err:
 void qmp_netdev_del(const char *id, Error **errp)
 {
     NetClientState *nc;
+    QemuOptsList *opt;
 
     nc = qemu_find_netdev(id);
     if (!nc) {
@@ -835,7 +836,12 @@ void qmp_netdev_del(const char *id, Error **errp)
     }
 
     qemu_del_net_client(nc);
-    qemu_opts_del(qemu_opts_find(qemu_find_opts_err("netdev", errp), id));
+    opt = qemu_find_opts_err("netdev", errp);
+    if (errp) {
+        error_setg(errp, "Failed to delete %s", id);
+        return;
+    }
+    qemu_opts_del(qemu_opts_find(opt, id));
 }
 
 void print_net_client(Monitor *mon, NetClientState *nc)
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] net: avoid segfault in case netdev_del non-host network device

[Stefan Hajnoczi]

On Thu, Nov 1, 2012 at 8:48 AM, Lei Li <lilei@linux.vnet.ibm.com> wrote:
> netdev_del assume that remove host network device.
> However, when try to remove a non-host network device
> by netdev_del, it will cause a segfault.

I recently sent a similar fix which forbids deleting non-netdev net
clients from netdev_del:
http://patchwork.ozlabs.org/patch/193759/

netdev_del should only be used on -netdev/netdev_add devices.
Therefore my patch raises an error before we call
qemu_del_net_client(nc).

Stefan

Re: [Qemu-devel] [PATCH] net: avoid segfault in case netdev_del non-host network device

[Lei Li]

On 11/01/2012 05:42 PM, Stefan Hajnoczi wrote:
> On Thu, Nov 1, 2012 at 8:48 AM, Lei Li <lilei@linux.vnet.ibm.com> wrote:
>> netdev_del assume that remove host network device.
>> However, when try to remove a non-host network device
>> by netdev_del, it will cause a segfault.
> I recently sent a similar fix which forbids deleting non-netdev net
> clients from netdev_del:
> http://patchwork.ozlabs.org/patch/193759/

Hi Stefan,

Sorry I did not see it... Seems I miss this chance to submit a patch.  :-P

BTW, I was thinking that should we add another hacking to check
if the deleting object is a netdev or a VLAN client?

>
> netdev_del should only be used on -netdev/netdev_add devices.
> Therefore my patch raises an error before we call
> qemu_del_net_client(nc).
>
> Stefan
>


-- 
Lei

Re: [Qemu-devel] [PATCH] net: avoid segfault in case netdev_del non-host network device

[Stefan Hajnoczi]

On Fri, Nov 2, 2012 at 3:10 AM, Lei Li <lilei@linux.vnet.ibm.com> wrote:
> On 11/01/2012 05:42 PM, Stefan Hajnoczi wrote:
>>
>> On Thu, Nov 1, 2012 at 8:48 AM, Lei Li <lilei@linux.vnet.ibm.com> wrote:
>>>
>>> netdev_del assume that remove host network device.
>>> However, when try to remove a non-host network device
>>> by netdev_del, it will cause a segfault.
>>
>> I recently sent a similar fix which forbids deleting non-netdev net
>> clients from netdev_del:
>> http://patchwork.ozlabs.org/patch/193759/
>
>
> Hi Stefan,
>
> Sorry I did not see it... Seems I miss this chance to submit a patch.  :-P
>
> BTW, I was thinking that should we add another hacking to check
> if the deleting object is a netdev or a VLAN client?

netdev_del now only deletes -netdev or netdev_add net clients.  It
refuses to delete -net clients because they are not in the "netdev"
QemuOptsList.  It also refuses to delete net/hub.c ports that were
added by net_hub_add_port() because they are not in the "netdev"
QemuOptsList.

I'm not sure I understand what you are suggesting?

Stefan