| Message ID | 20170527035115.6598-1-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
Thanks, - Liran ovs-dev-bounces@openvswitch.org wrote on 27/05/2017 06:51:15 AM: > From: Ben Pfaff <blp@ovn.org> > To: dev@openvswitch.org > Cc: Ben Pfaff <blp@ovn.org> > Date: 27/05/2017 06:51 AM > Subject: [ovs-dev] [PATCH] ovsdb: Check null before deref in > ovsdb_monitor_table_condition_update(). > Sent by: ovs-dev-bounces@openvswitch.org > > I believe that this would trigger an ovsdb-server crash if a client created > a plain RFC 7047 "monitor" and later attempted to update its condition. > > Found by Coverity. > > Reported-at: https://scan3.coverity.com/reports.htm#v16889/p10449/ > fileInstanceId=14763017&defectInstanceId=4305336&mergedDefectId=180412 > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > ovsdb/monitor.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > index 7e6ddcb2aa0b..b98100703091 100644 > --- a/ovsdb/monitor.c > +++ b/ovsdb/monitor.c > @@ -1,5 +1,5 @@ > /* > - * Copyright (c) 2015 Nicira, Inc. > + * Copyright (c) 2015, 2017 Nicira, Inc. > * > * Licensed under the Apache License, Version 2.0 (the "License"); > * you may not use this file except in compliance with the License. > @@ -687,15 +687,15 @@ ovsdb_monitor_table_condition_update( > const struct ovsdb_table *table, > const struct json *cond_json) > { > + if (!condition) { > + return NULL; > + } > + > struct ovsdb_monitor_table_condition *mtc = > shash_find_data(&condition->tables, table->schema->name); > struct ovsdb_error *error; > struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER(&cond); > > - if (!condition) { > - return NULL; > - } > - > error = ovsdb_condition_from_json(table->schema, cond_json, > NULL, &cond); > if (error) { > -- > 2.10.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
I guess that I will interpret that as a review. Thanks! I applied this to master, branch-2.7, and branch-2.6. On Mon, May 29, 2017 at 09:59:57AM +0300, Liran Schour wrote: > Thanks, > > - Liran > > ovs-dev-bounces@openvswitch.org wrote on 27/05/2017 06:51:15 AM: > > > From: Ben Pfaff <blp@ovn.org> > > To: dev@openvswitch.org > > Cc: Ben Pfaff <blp@ovn.org> > > Date: 27/05/2017 06:51 AM > > Subject: [ovs-dev] [PATCH] ovsdb: Check null before deref in > > ovsdb_monitor_table_condition_update(). > > Sent by: ovs-dev-bounces@openvswitch.org > > > > I believe that this would trigger an ovsdb-server crash if a client > created > > a plain RFC 7047 "monitor" and later attempted to update its condition. > > > > Found by Coverity. > > > > Reported-at: https://scan3.coverity.com/reports.htm#v16889/p10449/ > > fileInstanceId=14763017&defectInstanceId=4305336&mergedDefectId=180412 > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > ovsdb/monitor.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c > > index 7e6ddcb2aa0b..b98100703091 100644 > > --- a/ovsdb/monitor.c > > +++ b/ovsdb/monitor.c > > @@ -1,5 +1,5 @@ > > /* > > - * Copyright (c) 2015 Nicira, Inc. > > + * Copyright (c) 2015, 2017 Nicira, Inc. > > * > > * Licensed under the Apache License, Version 2.0 (the "License"); > > * you may not use this file except in compliance with the License. > > @@ -687,15 +687,15 @@ ovsdb_monitor_table_condition_update( > > const struct ovsdb_table *table, > > const struct json *cond_json) > > { > > + if (!condition) { > > + return NULL; > > + } > > + > > struct ovsdb_monitor_table_condition *mtc = > > shash_find_data(&condition->tables, table->schema->name); > > struct ovsdb_error *error; > > struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER(&cond); > > > > - if (!condition) { > > - return NULL; > > - } > > - > > error = ovsdb_condition_from_json(table->schema, cond_json, > > NULL, &cond); > > if (error) { > > -- > > 2.10.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > >
diff --git a/ovsdb/monitor.c b/ovsdb/monitor.c index 7e6ddcb2aa0b..b98100703091 100644 --- a/ovsdb/monitor.c +++ b/ovsdb/monitor.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015 Nicira, Inc. + * Copyright (c) 2015, 2017 Nicira, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -687,15 +687,15 @@ ovsdb_monitor_table_condition_update( const struct ovsdb_table *table, const struct json *cond_json) { + if (!condition) { + return NULL; + } + struct ovsdb_monitor_table_condition *mtc = shash_find_data(&condition->tables, table->schema->name); struct ovsdb_error *error; struct ovsdb_condition cond = OVSDB_CONDITION_INITIALIZER(&cond); - if (!condition) { - return NULL; - } - error = ovsdb_condition_from_json(table->schema, cond_json, NULL, &cond); if (error) {
I believe that this would trigger an ovsdb-server crash if a client created a plain RFC 7047 "monitor" and later attempted to update its condition. Found by Coverity. Reported-at: https://scan3.coverity.com/reports.htm#v16889/p10449/fileInstanceId=14763017&defectInstanceId=4305336&mergedDefectId=180412 Signed-off-by: Ben Pfaff <blp@ovn.org> --- ovsdb/monitor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)