diff mbox

[ovs-dev,4/4] netdev: fix crash when interface option is changed to invalid value

Message ID AM2PR07MB1042F7662BB44A6EA3153C878AD30@AM2PR07MB1042.eurprd07.prod.outlook.com
State Accepted
Headers show

Commit Message

Zoltan Balogh June 30, 2017, 3:29 p.m. UTC 
When trying to modify an interface option (e.g. remote IP of a GRE port) to
an invalid value, the vswitchd does crash. For instance:
 ovs-vsctl add-br br0
 ovs-vsctl add-port br0 gre0 -- set interface gre0 type=gre \
           options:remote_ip=10.0.0.2
 ovs-vsctl set interface gre0 options:remote_ip=9.9.9

The bug is caused by trying to dereference a NULL pointer. It was introduced
by the commit 9fff138ec3a6. Before that, the NULL pointer was handled by the
VLOG_WARN_BUF macro.

Signed-off-by: Zoltán Balogh <zoltan.balogh@ericsson.com>
CC: Daniele Di Proietto <diproiettod@vmware.com>
Fixes: 9fff138ec3a6 ("netdev: Add 'errp' to set_config().")
---
 lib/netdev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Ben Pfaff July 12, 2017, 12:36 a.m. UTC | #1 
On Fri, Jun 30, 2017 at 03:29:40PM +0000, Zoltán Balogh wrote:
> When trying to modify an interface option (e.g. remote IP of a GRE port) to
> an invalid value, the vswitchd does crash. For instance:
>  ovs-vsctl add-br br0
>  ovs-vsctl add-port br0 gre0 -- set interface gre0 type=gre \
>            options:remote_ip=10.0.0.2
>  ovs-vsctl set interface gre0 options:remote_ip=9.9.9

Good catch, thanks.  I applied this to master and branch-2.7.
diff mbox

Patch

diff --git a/lib/netdev.c b/lib/netdev.c
index a7840a8..26e87a2 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -469,7 +469,11 @@  netdev_set_config(struct netdev *netdev, const struct smap *args, char **errp)
                           "%s: could not set configuration (%s)",
                           netdev_get_name(netdev), ovs_strerror(error));
             if (verbose_error) {
-                *errp = verbose_error;
+                if (errp) {
+                    *errp = verbose_error;
+                } else {
+                    free(verbose_error);
+                }
             }
         }
         return error;