summaryrefslogtreecommitdiff
blob: 5d0348f4dcde800822465eb0bd6201286604b8fa (plain)
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
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
From c5a76df793c638423e1388528dc679a3e020a477 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:12 +0200
Subject: [PATCH 75/87] tools/xenstore: simplify check_store()

check_store() is using a hash table for storing all node names it has
found via walking the tree. Additionally it using another hash table
for all children of a node to detect duplicate child names.

Simplify that by dropping the second hash table as the first one is
already holding all the needed information.

This is part of XSA-418 / CVE-2022-42321.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 70f719f52a220bc5bc987e4dd28e14a7039a176b)
---
 tools/xenstore/xenstored_core.c | 47 +++++++++++----------------------
 1 file changed, 15 insertions(+), 32 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 2cda3ee375ab..760f3c16c794 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2477,50 +2477,34 @@ static int check_store_(const char *name, struct hashtable *reachable)
 	if (node) {
 		size_t i = 0;
 
-		struct hashtable * children =
-			create_hashtable(16, hash_from_key_fn, keys_equal_fn);
-		if (!children) {
-			log("check_store create table: ENOMEM");
-			return ENOMEM;
-		}
-
 		if (!remember_string(reachable, name)) {
-			hashtable_destroy(children, 0);
 			log("check_store: ENOMEM");
 			return ENOMEM;
 		}
 
 		while (i < node->childlen && !ret) {
-			struct node *childnode;
+			struct node *childnode = NULL;
 			size_t childlen = strlen(node->children + i);
-			char * childname = child_name(NULL, node->name,
-						      node->children + i);
+			char *childname = child_name(NULL, node->name,
+						     node->children + i);
 
 			if (!childname) {
 				log("check_store: ENOMEM");
 				ret = ENOMEM;
 				break;
 			}
+
+			if (hashtable_search(reachable, childname)) {
+				log("check_store: '%s' is duplicated!",
+				    childname);
+				i = rm_child_entry(node, i, childlen);
+				goto next;
+			}
+
 			childnode = read_node(NULL, childname, childname);
-			
+
 			if (childnode) {
-				if (hashtable_search(children, childname)) {
-					log("check_store: '%s' is duplicated!",
-					    childname);
-					i = rm_child_entry(node, i, childlen);
-				}
-				else {
-					if (!remember_string(children,
-							     childname)) {
-						log("check_store: ENOMEM");
-						talloc_free(childnode);
-						talloc_free(childname);
-						ret = ENOMEM;
-						break;
-					}
-					ret = check_store_(childname,
-							   reachable);
-				}
+				ret = check_store_(childname, reachable);
 			} else if (errno != ENOMEM) {
 				log("check_store: No child '%s' found!\n",
 				    childname);
@@ -2530,19 +2514,18 @@ static int check_store_(const char *name, struct hashtable *reachable)
 				ret = ENOMEM;
 			}
 
+ next:
 			talloc_free(childnode);
 			talloc_free(childname);
 			i += childlen + 1;
 		}
 
-		hashtable_destroy(children, 0 /* Don't free values (they are
-						 all (void *)1) */);
 		talloc_free(node);
 	} else if (errno != ENOMEM) {
 		/* Impossible, because no database should ever be without the
 		   root, and otherwise, we've just checked in our caller
 		   (which made a recursive call to get here). */
-		   
+
 		log("check_store: No child '%s' found: impossible!", name);
 	} else {
 		log("check_store: ENOMEM");
-- 
2.37.4