1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
|
commit 863f195fa823c0c20d734dadfc5908c2aea34330
Author: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Date: 2019-08-05 20:03:06 +0300
make check-unit: use after free in test-opts-visitor
In the struct OptsVisitor, the 'repeated_opts' member points to a list
in the 'unprocessed_opts' hash table after the list has been destroyed.
A subsequent call to visit_type_int() references the deleted list.
It results in use-after-free issue reproduced by running the test case
under the Valgrind: valgrind tests/test-opts-visitor.
A new mode ListMode::LM_TRAVERSED is declared to mark the list
traversal completed.
Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-Id: <1565024586-387112-1-git-send-email-andrey.shinkevich@virtuozzo.com>
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 324b197495a0..5fe0276c1cc8 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -24,7 +24,8 @@ enum ListMode
{
LM_NONE, /* not traversing a list of repeated options */
- LM_IN_PROGRESS, /* opts_next_list() ready to be called.
+ LM_IN_PROGRESS, /*
+ * opts_next_list() ready to be called.
*
* Generating the next list link will consume the most
* recently parsed QemuOpt instance of the repeated
@@ -36,7 +37,8 @@ enum ListMode
* LM_UNSIGNED_INTERVAL.
*/
- LM_SIGNED_INTERVAL, /* opts_next_list() has been called.
+ LM_SIGNED_INTERVAL, /*
+ * opts_next_list() has been called.
*
* Generating the next list link will consume the most
* recently stored element from the signed interval,
@@ -48,7 +50,14 @@ enum ListMode
* next element of the signed interval.
*/
- LM_UNSIGNED_INTERVAL /* Same as above, only for an unsigned interval. */
+ LM_UNSIGNED_INTERVAL, /* Same as above, only for an unsigned interval. */
+
+ LM_TRAVERSED /*
+ * opts_next_list() has been called.
+ *
+ * No more QemuOpt instance in the list.
+ * The traversal has been completed.
+ */
};
typedef enum ListMode ListMode;
@@ -238,6 +247,8 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
OptsVisitor *ov = to_ov(v);
switch (ov->list_mode) {
+ case LM_TRAVERSED:
+ return NULL;
case LM_SIGNED_INTERVAL:
case LM_UNSIGNED_INTERVAL:
if (ov->list_mode == LM_SIGNED_INTERVAL) {
@@ -258,6 +269,8 @@ opts_next_list(Visitor *v, GenericList *tail, size_t size)
opt = g_queue_pop_head(ov->repeated_opts);
if (g_queue_is_empty(ov->repeated_opts)) {
g_hash_table_remove(ov->unprocessed_opts, opt->name);
+ ov->repeated_opts = NULL;
+ ov->list_mode = LM_TRAVERSED;
return NULL;
}
break;
@@ -289,7 +302,8 @@ opts_end_list(Visitor *v, void **obj)
assert(ov->list_mode == LM_IN_PROGRESS ||
ov->list_mode == LM_SIGNED_INTERVAL ||
- ov->list_mode == LM_UNSIGNED_INTERVAL);
+ ov->list_mode == LM_UNSIGNED_INTERVAL ||
+ ov->list_mode == LM_TRAVERSED);
ov->repeated_opts = NULL;
ov->list_mode = LM_NONE;
}
@@ -306,6 +320,10 @@ lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp)
list = lookup_distinct(ov, name, errp);
return list ? g_queue_peek_tail(list) : NULL;
}
+ if (ov->list_mode == LM_TRAVERSED) {
+ error_setg(errp, "Fewer list elements than expected");
+ return NULL;
+ }
assert(ov->list_mode == LM_IN_PROGRESS);
return g_queue_peek_head(ov->repeated_opts);
}
|
