author | Lars Hjemli <hjemli@gmail.com> | 2006-12-12 09:16:41 (UTC) |
---|---|---|
committer | Lars Hjemli <hjemli@gmail.com> | 2006-12-12 09:16:41 (UTC) |
commit | 58d04f6523b0029281d65f841859fa42d0c744ff (patch) (unidiff) | |
tree | ed52e95047ccbb99152f7d3f009e57687e6452f1 | |
parent | fbaf1171b4e343929dd43ecac7cd9d1c692b84ec (diff) | |
download | cgit-58d04f6523b0029281d65f841859fa42d0c744ff.zip cgit-58d04f6523b0029281d65f841859fa42d0c744ff.tar.gz cgit-58d04f6523b0029281d65f841859fa42d0c744ff.tar.bz2 |
cache_lock: do xstrdup/free on lockfile
Since fmt() uses 8 alternating static buffers, and cache_lock might call
cache_create_dirs() multiple times, which in turn might call fmt() twice,
after four iterations lockfile would be overwritten by a cachedirectory
path.
In worst case, this could cause the cachedirectory to be unlinked and replaced
by a cachefile.
Fix: use xstrdup() on the result from fmt() before assigning to lockfile, and
call free(lockfile) before exit.
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
-rw-r--r-- | cache.c | 3 |
1 files changed, 2 insertions, 1 deletions
@@ -65,40 +65,41 @@ int cache_refill_overdue(const char *lockfile) | |||
65 | { | 65 | { |
66 | struct stat st; | 66 | struct stat st; |
67 | 67 | ||
68 | if (stat(lockfile, &st)) | 68 | if (stat(lockfile, &st)) |
69 | return 0; | 69 | return 0; |
70 | else | 70 | else |
71 | return (time(NULL) - st.st_mtime > cgit_cache_max_create_time); | 71 | return (time(NULL) - st.st_mtime > cgit_cache_max_create_time); |
72 | } | 72 | } |
73 | 73 | ||
74 | int cache_lock(struct cacheitem *item) | 74 | int cache_lock(struct cacheitem *item) |
75 | { | 75 | { |
76 | int i = 0; | 76 | int i = 0; |
77 | char *lockfile = fmt("%s.lock", item->name); | 77 | char *lockfile = xstrdup(fmt("%s.lock", item->name)); |
78 | 78 | ||
79 | top: | 79 | top: |
80 | if (++i > cgit_max_lock_attempts) | 80 | if (++i > cgit_max_lock_attempts) |
81 | die("cache_lock: unable to lock %s: %s", | 81 | die("cache_lock: unable to lock %s: %s", |
82 | item->name, strerror(errno)); | 82 | item->name, strerror(errno)); |
83 | 83 | ||
84 | item->fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR); | 84 | item->fd = open(lockfile, O_WRONLY|O_CREAT|O_EXCL, S_IRUSR|S_IWUSR); |
85 | 85 | ||
86 | if (item->fd == NOLOCK && errno == ENOENT && cache_create_dirs()) | 86 | if (item->fd == NOLOCK && errno == ENOENT && cache_create_dirs()) |
87 | goto top; | 87 | goto top; |
88 | 88 | ||
89 | if (item->fd == NOLOCK && errno == EEXIST && | 89 | if (item->fd == NOLOCK && errno == EEXIST && |
90 | cache_refill_overdue(lockfile) && !unlink(lockfile)) | 90 | cache_refill_overdue(lockfile) && !unlink(lockfile)) |
91 | goto top; | 91 | goto top; |
92 | 92 | ||
93 | free(lockfile); | ||
93 | return (item->fd > 0); | 94 | return (item->fd > 0); |
94 | } | 95 | } |
95 | 96 | ||
96 | int cache_unlock(struct cacheitem *item) | 97 | int cache_unlock(struct cacheitem *item) |
97 | { | 98 | { |
98 | close(item->fd); | 99 | close(item->fd); |
99 | return (rename(fmt("%s.lock", item->name), item->name) == 0); | 100 | return (rename(fmt("%s.lock", item->name), item->name) == 0); |
100 | } | 101 | } |
101 | 102 | ||
102 | int cache_cancel_lock(struct cacheitem *item) | 103 | int cache_cancel_lock(struct cacheitem *item) |
103 | { | 104 | { |
104 | return (unlink(fmt("%s.lock", item->name)) == 0); | 105 | return (unlink(fmt("%s.lock", item->name)) == 0); |