On Wed, Dec 20, 2017 at 03:03:37PM -0800, Justin Pettit wrote: > > > > On Dec 8, 2017, at 1:01 PM, Ben Pfaff <[email protected]> wrote: > > > > This oversight allowed monitor IDs to be duplicated when the > > monitor_cond_change request changed them. > > > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > ovsdb/jsonrpc-server.c | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c > > index da3b3835f0d7..36e50241b040 100644 > > --- a/ovsdb/jsonrpc-server.c > > +++ b/ovsdb/jsonrpc-server.c > > @@ -1420,6 +1420,14 @@ ovsdb_jsonrpc_monitor_cond_change(struct > > ovsdb_jsonrpc_session *s, > > goto error; > > } > > > > + struct ovsdb_jsonrpc_monitor *m2 > > + = ovsdb_jsonrpc_monitor_find(s, params->u.array.elems[1]); > > + if (m2 && m2 != m) { > > + error = ovsdb_syntax_error(params->u.array.elems[1], NULL, > > + "duplicate monitor ID"); > > + goto error; > > + } > > As we discussed off-line, I found the logic a bit confusing. It might be > clearer with either a comment or a slight restructuring of the code. > > Acked-by: Justin Pettit <[email protected]>
Thanks. What do you think of this version? --8<--------------------------cut here-------------------------->8-- From: Ben Pfaff <[email protected]> Date: Thu, 21 Dec 2017 16:29:32 -0800 Subject: [PATCH] jsonrpc-server: Enforce uniqueness of monitor IDs. This oversight allowed monitor IDs to be duplicated when the monitor_cond_change request changed them. Signed-off-by: Ben Pfaff <[email protected]> Acked-by: Justin Pettit <[email protected]> --- ovsdb/jsonrpc-server.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c index f4a14d4fb2c2..07e8d1f17b51 100644 --- a/ovsdb/jsonrpc-server.c +++ b/ovsdb/jsonrpc-server.c @@ -1411,6 +1411,14 @@ ovsdb_jsonrpc_monitor_cond_change(struct ovsdb_jsonrpc_session *s, goto error; } + const struct json *new_monitor_id = params->u.array.elems[1]; + bool changing_id = !json_equal(m->monitor_id, new_monitor_id); + if (changing_id && ovsdb_jsonrpc_monitor_find(s, new_monitor_id)) { + error = ovsdb_syntax_error(new_monitor_id, NULL, + "duplicate monitor ID"); + goto error; + } + monitor_cond_change_reqs = params->u.array.elems[2]; if (monitor_cond_change_reqs->type != JSON_OBJECT) { error = @@ -1458,10 +1466,12 @@ ovsdb_jsonrpc_monitor_cond_change(struct ovsdb_jsonrpc_session *s, } /* Change monitor id */ - hmap_remove(&s->monitors, &m->node); - json_destroy(m->monitor_id); - m->monitor_id = json_clone(params->u.array.elems[1]); - hmap_insert(&s->monitors, &m->node, json_hash(m->monitor_id, 0)); + if (changing_id) { + hmap_remove(&s->monitors, &m->node); + json_destroy(m->monitor_id); + m->monitor_id = json_clone(new_monitor_id); + hmap_insert(&s->monitors, &m->node, json_hash(m->monitor_id, 0)); + } /* Send the new update, if any, represents the difference from the old * condition and the new one. */ -- 2.10.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
