aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPeter Seebach <peter.seebach@windriver.com>2011-01-13 17:30:53 -0600
committerPeter Seebach <peter.seebach@windriver.com>2011-01-13 17:30:53 -0600
commit2bac1363e4268c188b9d078dd8e8bbe944f524de (patch)
treeb6ce2931dc889590e5db4fae0d3475b084bc6b51
parent68659fda90550dbe0d1924a7b4ccbaa406d24257 (diff)
downloadpseudo-2bac1363e4268c188b9d078dd8e8bbe944f524de.tar.gz
pseudo-2bac1363e4268c188b9d078dd8e8bbe944f524de.tar.bz2
pseudo-2bac1363e4268c188b9d078dd8e8bbe944f524de.zip
Fixup: The path code could double-free.
The problem is that path_by_ino could end up being the same pointer as cache_path, after which, if cache_path were freed (or kept around for later), there would be malloc arena problems. Also, fix the calculation for pathlen to increase cache hits. The IPC messages use length of path *plus one* as the length, because the buffer is defined to include its terminating null byte.
-rw-r--r--pseudo.c41
1 files changed, 31 insertions, 10 deletions
diff --git a/pseudo.c b/pseudo.c
index 9fc49f6..efee436 100644
--- a/pseudo.c
+++ b/pseudo.c
@@ -522,6 +522,7 @@ pseudo_op(pseudo_msg_t *msg, const char *program, const char *tag) {
/* no need to restore msg */
found_path = 1;
found_ino = 1;
+ /* note: we have to avoid freeing this later */
path_by_ino = msg->path;
} else if (!pdb_find_file_exact(msg)) {
/* restore header contents */
@@ -1021,30 +1022,50 @@ pseudo_op(pseudo_msg_t *msg, const char *program, const char *tag) {
msg->op != OP_DID_UNLINK &&
msg->op != OP_CANCEL_UNLINK &&
msg->op != OP_UNLINK) {
-
cache_msg = *msg;
- free(cache_path);
-
- if (path_by_ino)
- cache_path = strdup(path_by_ino);
- else
+ if (path_by_ino) {
+ /* if path_by_ino is already the cache path,
+ * no need to dup it.
+ */
+ if (path_by_ino != cache_path) {
+ free(cache_path);
+ cache_path = strdup(path_by_ino);
+ } else {
+ /* don't want to free this */
+ path_by_ino = NULL;
+ }
+ } else {
+ free(cache_path);
cache_path = strdup(msg->path);
-
- cache_msg.pathlen = strlen(cache_path);
+ }
+ if (cache_path) {
+ cache_msg.pathlen = strlen(cache_path) + 1;
+ } else {
+ cache_msg.pathlen = 0;
+ }
} else {
cache_msg.pathlen = 0;
cache_msg.dev = 0;
cache_msg.ino = 0;
+ /* if path_by_ino was the same pointer,
+ * mark it NULL so we don't try to free
+ * it.
+ */
+ if (path_by_ino == cache_path)
+ path_by_ino = NULL;
free(cache_path);
cache_path = NULL;
}
/* in the case of an exact match, we just used the pointer
- * rather than allocating space
+ * rather than allocating space. If we used the cached path,
+ * and this pointer has already been invalidated, it'll be
+ * NULL now.
*/
- if (path_by_ino != msg->path)
+ if (path_by_ino != msg->path) {
free(path_by_ino);
+ }
pseudo_debug(2, "completed %s.\n", pseudo_op_name(msg->op));
if (opt_l)
pdb_log_msg(SEVERITY_INFO, msg, program, tag, NULL);