On Sat, Apr 16, 2016 at 09:28:40PM -0700, Jarno Rajahalme wrote:
> Addition of table versioning exposed struct cls_rule member
> 'cls_match' to RCU readers and made it possible for
> 'cls_match' become NULL while being accessed by an RCU reader, but we failed
> to check for this condition. This may have resulted in NULL pointer
> dereference and ovs-vswitchd crash.
>
> Fix this by making the 'cls_match' member an RCU pointer and checking
> the value whenever it potentially read by an RCU reader. In these
> instances we use ovsrcu_get(), whereas functions accessible only by
> the exclusive writers use ovsrcu_get_protected() and do not need to
> check the result.
>
> VMware-BZ: 1643642
> Fixes: 2b7b1427 ("classifier: Support table versioning")
> Signed-off-by: Jarno Rajahalme <ja...@ovn.org>
Thank you for finding this!
I read the code carefully. I also built it (on top of master) with
Clang 3.5 and GCC 4.9.1 (for i386), getting clean compiles and no
warnings from "sparse" either. I also ran the testsuite and it passed.
I think that most of the changes to classifier.h are just reordering the
function prototypes. I don't know why this order is being changed.
Maybe it is to group the comments better? I don't know whether the
reordering is properly part of a minimal fix (since this will require
backporting); I'll leave that to your judgment.
This new comment in classifier.h could use a space at the end:
/* Classifier rule properties. These are RCU protected and may run
* concurrently with modifiers and each other.*/
I think that the change to classifier_rule_overlaps() causes it to
inline cls_rule_visible_in_version(), so that it could be made clearer
by just calling that function directly.
The number of instances, with or without _protected, of
ovsrcu_get(struct cls_match *, &rule->cls_match)
verges on wanting a pair of helper functions, especially since it would
probably reduce the amount of line-breaking. Again I leave it to your
judgment.
Acked-by: Ben Pfaff <b...@ovn.org>
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
