diff mbox series

[ovs-dev] use double postponing to free subtables.

Message ID 1555321001-12728-1-git-send-email-fuzhantao@huawei.com
State Superseded
Headers show
Series [ovs-dev] use double postponing to free subtables. | expand

Commit Message

fuzhantao April 15, 2019, 9:36 a.m. UTC 
From: Zhantao Fu <fuzhantao@huawei.com>

Now the subtable in classifier/dpcls is postponed by destroy_subtable before move itself out of cls->subtables(pvector).It will raise use-after-free problem.

use double postponing to free subtable

Signed-off-by: Zhantao Fu <fuzhantao@huawei.com>
---
 lib/classifier.c  | 29 +++++++++++++++++++----------
 lib/dpif-netdev.c | 10 ++++++++--
 2 files changed, 27 insertions(+), 12 deletions(-)

Comments

0-day Robot April 15, 2019, 10:58 a.m. UTC | #1 
Bleep bloop.  Greetings fuzhantao, 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.


build:
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/bundle.lo -MD -MP -MF $depbase.Tpo -c -o lib/bundle.lo lib/bundle.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/bundle.lo -MD -MP -MF lib/.deps/bundle.Tpo -c lib/bundle.c -o lib/bundle.o
depbase=`echo lib/byteq.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/byteq.lo -MD -MP -MF $depbase.Tpo -c -o lib/byteq.lo lib/byteq.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/byteq.lo -MD -MP -MF lib/.deps/byteq.Tpo -c lib/byteq.c -o lib/byteq.o
depbase=`echo lib/cfm.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/cfm.lo -MD -MP -MF $depbase.Tpo -c -o lib/cfm.lo lib/cfm.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/cfm.lo -MD -MP -MF lib/.deps/cfm.Tpo -c lib/cfm.c -o lib/cfm.o
depbase=`echo lib/classifier.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror   -g -O2 -MT lib/classifier.lo -MD -MP -MF $depbase.Tpo -c -o lib/classifier.lo lib/classifier.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Werror -Werror -g -O2 -MT lib/classifier.lo -MD -MP -MF lib/.deps/classifier.Tpo -c lib/classifier.c -o lib/classifier.o
lib/classifier.c: In function 'destroy_subtable':
lib/classifier.c:1555:9: error: unused variable 'i' [-Werror=unused-variable]
     int i;
         ^
cc1: all warnings being treated as errors
make[2]: *** [lib/classifier.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


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

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/lib/classifier.c b/lib/classifier.c
index edb40c8..10370ce 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1468,6 +1468,24 @@  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;
+
+    ovs_assert(ovsrcu_get_protected(struct trie_node *, &subtable->ports_trie)
+               == NULL);
+    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);
+}
+
 /* 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)
@@ -1540,16 +1558,7 @@  destroy_subtable(struct classifier *cls, struct cls_subtable *subtable)
     cmap_remove(&cls->subtables_map, &subtable->cmap_node,
                 minimask_hash(&subtable->mask, 0));
 
-    ovs_assert(ovsrcu_get_protected(struct trie_node *, &subtable->ports_trie)
-               == NULL);
-    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);
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4d6d0c3..8657a61 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -7592,6 +7592,13 @@  struct dpcls_subtable {
     /* 'mask' must be the last field, additional space is allocated here. */
 };
 
+static void
+dpcls_subtable_destroy_cb(struct dpcls_subtable *subtable)
+{
+    cmap_destroy(&subtable->rules);
+    ovsrcu_postpone(free, subtable);
+}
+
 /* Initializes 'cls' as a classifier that initially contains no classification
  * rules. */
 static void
@@ -7608,8 +7615,7 @@  dpcls_destroy_subtable(struct dpcls *cls, struct dpcls_subtable *subtable)
     pvector_remove(&cls->subtables, subtable);
     cmap_remove(&cls->subtables_map, &subtable->cmap_node,
                 subtable->mask.hash);
-    cmap_destroy(&subtable->rules);
-    ovsrcu_postpone(free, subtable);
+    ovsrcu_postpone(dpcls_subtable_destroy_cb, subtable);
 }
 
 /* Destroys 'cls'.  Rules within 'cls', if any, are not freed; this is the