[ovs-dev] classifier: Avoid use-after-free due to classifier_destroy().

Message ID 20190412224209.3101-1-blp@ovn.org
State New
Headers show
Series
  • [ovs-dev] classifier: Avoid use-after-free due to classifier_destroy().
Related show

Commit Message

Ben Pfaff April 12, 2019, 10:42 p.m.
From: fuzhantao <fuzhantao@huawei.com>

classifier_destroy() didn't postpone destroying subtables enough.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
I need a Signed-off-by from fuzhantao.

 lib/classifier.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comments

0-day Robot April 13, 2019, 12:02 a.m. | #1
Bleep bloop.  Greetings Ben Pfaff, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author fuzhantao <fuzhantao@huawei.com> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Ben Pfaff <blp@ovn.org>
Lines checked: 68, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot

Patch

diff --git a/lib/classifier.c b/lib/classifier.c
index edb40c89c608..595d2fdf54c3 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1468,6 +1468,19 @@  miniflow_get_map_in_range(const struct miniflow *miniflow, uint8_t start,
     return map;
 }
 
+static void
+subtable_destroy_cb(struct cls_subtable *subtable)
+{
+    int i;
+
+    for (i = 0; i < subtable->n_indices; i++) {
+        ccmap_destroy(&subtable->indices[i]);
+    }
+    cmap_destroy(&subtable->rules);
+
+    ovsrcu_postpone(free, subtable);
+}
+
 /* The new subtable will be visible to the readers only after this. */
 static struct cls_subtable *
 insert_subtable(struct classifier *cls, const struct minimask *mask)
@@ -1530,12 +1543,11 @@  insert_subtable(struct classifier *cls, const struct minimask *mask)
     return subtable;
 }
 
-/* RCU readers may still access the subtable before it is actually freed. */
+/* RCU readers may still access the subtable before it is actually freed.
+ * double postpone for subtable to avoid use-after-free. */
 static void
 destroy_subtable(struct classifier *cls, struct cls_subtable *subtable)
 {
-    int i;
-
     pvector_remove(&cls->subtables, subtable);
     cmap_remove(&cls->subtables_map, &subtable->cmap_node,
                 minimask_hash(&subtable->mask, 0));
@@ -1545,11 +1557,7 @@  destroy_subtable(struct classifier *cls, struct cls_subtable *subtable)
     ovs_assert(cmap_is_empty(&subtable->rules));
     ovs_assert(rculist_is_empty(&subtable->rules_list));
 
-    for (i = 0; i < subtable->n_indices; i++) {
-        ccmap_destroy(&subtable->indices[i]);
-    }
-    cmap_destroy(&subtable->rules);
-    ovsrcu_postpone(free, subtable);
+    ovsrcu_postpone(subtable_destroy_cb, subtable);
 }
 
 static unsigned int be_get_bit_at(const ovs_be32 value[], unsigned int ofs);