| Message ID | 20161221070309.27374-2-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
Acked-by: Jarno Rajahalme <jarno@ovn.org> > On Dec 20, 2016, at 11:03 PM, Ben Pfaff <blp@ovn.org> wrote: > > The 'flow_cookie' member of struct rule is read during flow translation > without holding a mutex, breaking the documented OVS_GUARDED access rule. > However, the flow_cookie member is actually never modified after a rule is > initialized, so this commit changes the access rules to reflect that. > > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > ofproto/ofproto-provider.h | 4 ++-- > ofproto/ofproto.c | 5 +++-- > 2 files changed, 5 insertions(+), 4 deletions(-) > > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > index c515779..3739ebc 100644 > --- a/ofproto/ofproto-provider.h > +++ b/ofproto/ofproto-provider.h > @@ -365,8 +365,8 @@ struct rule { > struct ovs_refcount ref_count; > > /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with > - * a flow.. */ > - ovs_be64 flow_cookie OVS_GUARDED; > + * a flow. */ > + const ovs_be64 flow_cookie; /* Immutable once rule is constructed. */ > struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex); > > enum ofputil_flow_mod_flags flags OVS_GUARDED; > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > index 4f43c45..58295a1 100644 > --- a/ofproto/ofproto.c > +++ b/ofproto/ofproto.c > @@ -4859,7 +4859,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct cls_rule *cr, > > ovs_mutex_init(&rule->mutex); > ovs_mutex_lock(&rule->mutex); > - rule->flow_cookie = new_cookie; > + *CONST_CAST(ovs_be64 *, &rule->flow_cookie) = new_cookie; > rule->created = rule->modified = time_msec(); > rule->idle_timeout = idle_timeout; > rule->hard_timeout = hard_timeout; > @@ -5098,7 +5098,8 @@ replace_rule_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm, > new_rule->created = old_rule->created; > } > if (!change_cookie) { > - new_rule->flow_cookie = old_rule->flow_cookie; > + *CONST_CAST(ovs_be64 *, &new_rule->flow_cookie) > + = old_rule->flow_cookie; > } > ovs_mutex_unlock(&old_rule->mutex); > ovs_mutex_unlock(&new_rule->mutex); > -- > 2.10.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Thanks! I applied this patch to master. On Wed, Dec 21, 2016 at 10:40:23AM -0800, Jarno Rajahalme wrote: > Acked-by: Jarno Rajahalme <jarno@ovn.org> > > > On Dec 20, 2016, at 11:03 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > The 'flow_cookie' member of struct rule is read during flow translation > > without holding a mutex, breaking the documented OVS_GUARDED access rule. > > However, the flow_cookie member is actually never modified after a rule is > > initialized, so this commit changes the access rules to reflect that. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > ofproto/ofproto-provider.h | 4 ++-- > > ofproto/ofproto.c | 5 +++-- > > 2 files changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > > index c515779..3739ebc 100644 > > --- a/ofproto/ofproto-provider.h > > +++ b/ofproto/ofproto-provider.h > > @@ -365,8 +365,8 @@ struct rule { > > struct ovs_refcount ref_count; > > > > /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with > > - * a flow.. */ > > - ovs_be64 flow_cookie OVS_GUARDED; > > + * a flow. */ > > + const ovs_be64 flow_cookie; /* Immutable once rule is constructed. */ > > struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex); > > > > enum ofputil_flow_mod_flags flags OVS_GUARDED; > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > > index 4f43c45..58295a1 100644 > > --- a/ofproto/ofproto.c > > +++ b/ofproto/ofproto.c > > @@ -4859,7 +4859,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct cls_rule *cr, > > > > ovs_mutex_init(&rule->mutex); > > ovs_mutex_lock(&rule->mutex); > > - rule->flow_cookie = new_cookie; > > + *CONST_CAST(ovs_be64 *, &rule->flow_cookie) = new_cookie; > > rule->created = rule->modified = time_msec(); > > rule->idle_timeout = idle_timeout; > > rule->hard_timeout = hard_timeout; > > @@ -5098,7 +5098,8 @@ replace_rule_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm, > > new_rule->created = old_rule->created; > > } > > if (!change_cookie) { > > - new_rule->flow_cookie = old_rule->flow_cookie; > > + *CONST_CAST(ovs_be64 *, &new_rule->flow_cookie) > > + = old_rule->flow_cookie; > > } > > ovs_mutex_unlock(&old_rule->mutex); > > ovs_mutex_unlock(&new_rule->mutex); > > -- > > 2.10.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index c515779..3739ebc 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -365,8 +365,8 @@ struct rule { struct ovs_refcount ref_count; /* A "flow cookie" is the OpenFlow name for a 64-bit value associated with - * a flow.. */ - ovs_be64 flow_cookie OVS_GUARDED; + * a flow. */ + const ovs_be64 flow_cookie; /* Immutable once rule is constructed. */ struct hindex_node cookie_node OVS_GUARDED_BY(ofproto_mutex); enum ofputil_flow_mod_flags flags OVS_GUARDED; diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 4f43c45..58295a1 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -4859,7 +4859,7 @@ ofproto_rule_create(struct ofproto *ofproto, struct cls_rule *cr, ovs_mutex_init(&rule->mutex); ovs_mutex_lock(&rule->mutex); - rule->flow_cookie = new_cookie; + *CONST_CAST(ovs_be64 *, &rule->flow_cookie) = new_cookie; rule->created = rule->modified = time_msec(); rule->idle_timeout = idle_timeout; rule->hard_timeout = hard_timeout; @@ -5098,7 +5098,8 @@ replace_rule_start(struct ofproto *ofproto, struct ofproto_flow_mod *ofm, new_rule->created = old_rule->created; } if (!change_cookie) { - new_rule->flow_cookie = old_rule->flow_cookie; + *CONST_CAST(ovs_be64 *, &new_rule->flow_cookie) + = old_rule->flow_cookie; } ovs_mutex_unlock(&old_rule->mutex); ovs_mutex_unlock(&new_rule->mutex);
The 'flow_cookie' member of struct rule is read during flow translation without holding a mutex, breaking the documented OVS_GUARDED access rule. However, the flow_cookie member is actually never modified after a rule is initialized, so this commit changes the access rules to reflect that. Signed-off-by: Ben Pfaff <blp@ovn.org> --- ofproto/ofproto-provider.h | 4 ++-- ofproto/ofproto.c | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-)