Commit 836447e586f275a1f0b32860805f69ad6867ec32
2016-03-10T16:52:09
config_file: handle error when trying to lock strmap Accessing the current values map is handled through the `refcounder_strmap_take` function, which first acquires a mutex before accessing its values. While this assures everybody is trying to access the values with the mutex only we do not check if the locking actually succeeds. Fix the issue by checking if acquiring the lock succeeds and returning `NULL` if we encounter an error. Adjust callers.
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
diff --git a/src/config_file.c b/src/config_file.c
index 5f5e309..d79175a 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -232,7 +232,10 @@ static refcounted_strmap *refcounted_strmap_take(diskfile_header *h)
{
refcounted_strmap *map;
- git_mutex_lock(&h->values_mutex);
+ if (git_mutex_lock(&h->values_mutex) < 0) {
+ giterr_set(GITERR_OS, "Failed to lock config backend");
+ return NULL;
+ }
map = h->values;
git_atomic_inc(&map->refcount);
@@ -318,7 +321,10 @@ static int config__refresh(git_config_backend *cfg)
if ((error = config_read(values->values, b, reader, b->level, 0)) < 0)
goto out;
- git_mutex_lock(&b->header.values_mutex);
+ if ((error = git_mutex_lock(&b->header.values_mutex)) < 0) {
+ giterr_set(GITERR_OS, "Failed to lock config backend");
+ goto out;
+ }
tmp = b->header.values;
b->header.values = values;
@@ -460,7 +466,8 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val
if ((rval = git_config__normalize_name(name, &key)) < 0)
return rval;
- map = refcounted_strmap_take(&b->header);
+ if ((map = refcounted_strmap_take(&b->header)) == NULL)
+ return -1;
values = map->values;
/*
@@ -527,7 +534,8 @@ static int config_get(git_config_backend *cfg, const char *key, git_config_entry
if (!h->parent.readonly && ((error = config_refresh(cfg)) < 0))
return error;
- map = refcounted_strmap_take(h);
+ if ((map = refcounted_strmap_take(h)) == NULL)
+ return -1;
values = map->values;
pos = git_strmap_lookup_index(values, key);
@@ -565,7 +573,8 @@ static int config_set_multivar(
if ((result = git_config__normalize_name(name, &key)) < 0)
return result;
- map = refcounted_strmap_take(&b->header);
+ if ((map = refcounted_strmap_take(&b->header)) == NULL)
+ return -1;
values = b->header.values->values;
pos = git_strmap_lookup_index(values, key);
@@ -610,7 +619,8 @@ static int config_delete(git_config_backend *cfg, const char *name)
if ((result = git_config__normalize_name(name, &key)) < 0)
return result;
- map = refcounted_strmap_take(&b->header);
+ if ((map = refcounted_strmap_take(&b->header)) == NULL)
+ return -1;
values = b->header.values->values;
pos = git_strmap_lookup_index(values, key);
@@ -649,7 +659,8 @@ static int config_delete_multivar(git_config_backend *cfg, const char *name, con
if ((result = git_config__normalize_name(name, &key)) < 0)
return result;
- map = refcounted_strmap_take(&b->header);
+ if ((map = refcounted_strmap_take(&b->header)) == NULL)
+ return -1;
values = b->header.values->values;
pos = git_strmap_lookup_index(values, key);
@@ -832,7 +843,8 @@ static int config_readonly_open(git_config_backend *cfg, git_config_level_t leve
/* We're just copying data, don't care about the level */
GIT_UNUSED(level);
- src_map = refcounted_strmap_take(src_header);
+ if ((src_map = refcounted_strmap_take(src_header)) == NULL)
+ return -1;
b->header.values = src_map;
return 0;