1 |
commit: 3cd1221ff6fdd8b3af243f390569b6e61bfd9e18 |
2 |
Author: Fabian Groffen <grobian <AT> gentoo <DOT> org> |
3 |
AuthorDate: Sat Jun 12 20:03:13 2021 +0000 |
4 |
Commit: Fabian Groffen <grobian <AT> gentoo <DOT> org> |
5 |
CommitDate: Sat Jun 12 20:03:13 2021 +0000 |
6 |
URL: https://gitweb.gentoo.org/proj/portage-utils.git/commit/?id=3cd1221f |
7 |
|
8 |
libq/tree: fix double free/use after free scenarios |
9 |
|
10 |
- ensure we don't reuse pointers too many so we end up freeing the wrong |
11 |
thing |
12 |
- don't free atom and meta in case of cached pkg_ctx |
13 |
|
14 |
Signed-off-by: Fabian Groffen <grobian <AT> gentoo.org> |
15 |
|
16 |
libq/tree.c | 39 ++++++++++++++++++++------------------- |
17 |
libq/tree.h | 5 ++++- |
18 |
2 files changed, 24 insertions(+), 20 deletions(-) |
19 |
|
20 |
diff --git a/libq/tree.c b/libq/tree.c |
21 |
index 39beac8..a247f66 100644 |
22 |
--- a/libq/tree.c |
23 |
+++ b/libq/tree.c |
24 |
@@ -162,11 +162,11 @@ tree_open_binpkg(const char *sroot, const char *spkg) |
25 |
ret->cachetype = CACHE_BINPKGS; |
26 |
|
27 |
fd = openat(ret->tree_fd, binpkg_packages, O_RDONLY | O_CLOEXEC); |
28 |
- if (eat_file_fd(fd, &ret->pkgs, &ret->pkgslen)) { |
29 |
+ if (eat_file_fd(fd, &ret->cache.store, &ret->cache.storesize)) { |
30 |
ret->cachetype = CACHE_PACKAGES; |
31 |
- } else if (ret->pkgs != NULL) { |
32 |
- free(ret->pkgs); |
33 |
- ret->pkgs = NULL; |
34 |
+ } else if (ret->cache.store != NULL) { |
35 |
+ free(ret->cache.store); |
36 |
+ ret->cache.store = NULL; |
37 |
} |
38 |
close(fd); |
39 |
} |
40 |
@@ -1358,14 +1358,13 @@ tree_foreach_packages(tree_ctx *ctx, tree_pkg_cb callback, void *priv) |
41 |
depend_atom *atom = NULL; |
42 |
|
43 |
/* re-read the contents, this is necessary to make it possible to |
44 |
- * call this function multiple times |
45 |
- * TODO: generate an internal in-memory tree when cache is enabled */ |
46 |
- if (ctx->pkgs == NULL || ctx->pkgs[0] == '\0') { |
47 |
+ * call this function multiple times */ |
48 |
+ if (ctx->cache.store == NULL || ctx->cache.store[0] == '\0') { |
49 |
int fd = openat(ctx->tree_fd, binpkg_packages, O_RDONLY | O_CLOEXEC); |
50 |
- if (!eat_file_fd(fd, &ctx->pkgs, &ctx->pkgslen)) { |
51 |
- if (ctx->pkgs != NULL) { |
52 |
- free(ctx->pkgs); |
53 |
- ctx->pkgs = NULL; |
54 |
+ if (!eat_file_fd(fd, &ctx->cache.store, &ctx->cache.storesize)) { |
55 |
+ if (ctx->cache.store != NULL) { |
56 |
+ free(ctx->cache.store); |
57 |
+ ctx->cache.store = NULL; |
58 |
} |
59 |
close(fd); |
60 |
return 1; |
61 |
@@ -1373,8 +1372,8 @@ tree_foreach_packages(tree_ctx *ctx, tree_pkg_cb callback, void *priv) |
62 |
close(fd); |
63 |
} |
64 |
|
65 |
- p = ctx->pkgs; |
66 |
- len = strlen(ctx->pkgs); /* sucks, need eat_file change */ |
67 |
+ p = ctx->cache.store; |
68 |
+ len = strlen(ctx->cache.store); /* sucks, need eat_file change */ |
69 |
|
70 |
memset(&meta, 0, sizeof(meta)); |
71 |
|
72 |
@@ -1396,9 +1395,8 @@ tree_foreach_packages(tree_ctx *ctx, tree_pkg_cb callback, void *priv) |
73 |
|
74 |
memset(&pkg, 0, sizeof(pkg)); |
75 |
|
76 |
- /* store meta ptr in repo->pkgs, such that get_pkg_meta |
77 |
+ /* store meta ptr in ctx->pkgs, such that get_pkg_meta |
78 |
* can grab it from there (for free) */ |
79 |
- c = ctx->pkgs; |
80 |
ctx->pkgs = (char *)&meta; |
81 |
|
82 |
if (cat == NULL || strcmp(cat->name, atom->CATEGORY) != 0) |
83 |
@@ -1429,7 +1427,7 @@ tree_foreach_packages(tree_ctx *ctx, tree_pkg_cb callback, void *priv) |
84 |
/* do call callback with pkg_atom (populate cat and pkg) */ |
85 |
ret |= callback(&pkg, priv); |
86 |
|
87 |
- ctx->pkgs = c; |
88 |
+ ctx->pkgs = NULL; |
89 |
if (atom != (depend_atom *)cat->pkg_ctxs) |
90 |
atom_implode(atom); |
91 |
} |
92 |
@@ -1505,7 +1503,7 @@ tree_foreach_packages(tree_ctx *ctx, tree_pkg_cb callback, void *priv) |
93 |
/* ensure we don't free a garbage pointer */ |
94 |
ctx->repo = NULL; |
95 |
ctx->do_sort = false; |
96 |
- ctx->pkgs[0] = '\0'; |
97 |
+ ctx->cache.store[0] = '\0'; |
98 |
|
99 |
return ret; |
100 |
} |
101 |
@@ -1690,7 +1688,7 @@ tree_match_atom_cache_populate_cb(tree_pkg_ctx *ctx, void *priv) |
102 |
if (meta != NULL) { |
103 |
pkg->meta = xmalloc(sizeof(*pkg->meta)); |
104 |
memcpy(pkg->meta, meta, sizeof(*pkg->meta)); |
105 |
- pkg->meta->Q__data = NULL; /* avoid free here */ |
106 |
+ pkg->meta->Q__data = NULL; /* avoid free here (just to be sure) */ |
107 |
pkg->fd = -2; /* don't try to read, we already got it */ |
108 |
} else { |
109 |
pkg->meta = NULL; |
110 |
@@ -1788,6 +1786,9 @@ tree_match_atom(tree_ctx *ctx, depend_atom *query, int flags) |
111 |
C->ctx->cachetype == CACHE_PACKAGES ? ".tbz2" : ""); \ |
112 |
if (flags & TREE_MATCH_METADATA) \ |
113 |
n->meta = tree_pkg_read(pkg_ctx); \ |
114 |
+ if (C->ctx->cachetype == CACHE_BINPKGS || \ |
115 |
+ C->ctx->cachetype == CACHE_PACKAGES) \ |
116 |
+ n->free_atom = n->free_meta = 0; \ |
117 |
n->next = ret; \ |
118 |
ret = n; \ |
119 |
lastpn = atom->PN; \ |
120 |
@@ -1825,7 +1826,7 @@ tree_match_close(tree_match_ctx *match) |
121 |
w = match->next; |
122 |
if (match->free_atom) |
123 |
atom_implode(match->atom); |
124 |
- if (match->meta != NULL) |
125 |
+ if (match->free_meta && match->meta != NULL) |
126 |
tree_close_meta(match->meta); |
127 |
free(match); |
128 |
} |
129 |
|
130 |
diff --git a/libq/tree.h b/libq/tree.h |
131 |
index 8741bad..1f28205 100644 |
132 |
--- a/libq/tree.h |
133 |
+++ b/libq/tree.h |
134 |
@@ -47,6 +47,8 @@ struct tree_ctx { |
135 |
size_t pkgslen; |
136 |
depend_atom *query_atom; |
137 |
struct tree_cache { |
138 |
+ char *store; |
139 |
+ size_t storesize; |
140 |
set *categories; |
141 |
bool all_categories:1; |
142 |
} cache; |
143 |
@@ -126,7 +128,8 @@ struct tree_match_ctx { |
144 |
tree_pkg_meta *meta; |
145 |
char path[_Q_PATH_MAX + 48]; |
146 |
tree_match_ctx *next; |
147 |
- int free_atom; |
148 |
+ char free_atom:1; |
149 |
+ char free_meta:1; |
150 |
}; |
151 |
|
152 |
/* foreach pkg callback function signature */ |