@matheusbsbs / matheus-bagliano / commits / faf123c730
Fork 0

grep: fix race conditions at grep_submodule()

There're currently two function calls in builtin/grep.c:grep_submodule() which might result in race conditions: - submodule_from_path(): it has config_with_options() in its call stack which, in turn, may have read_object_file() in its own. Therefore, calling the first function without acquiring grep_read_mutex may end up causing a race condition with other object read operations performed by worker threads (for example, at the fill_textconv() call in grep.c:fill_textconv_grep()). - parse_object_or_die(): it falls into the same problem, having repo_has_object_file(the_repository, ...) in its call stack. Besides that, parse_object(), which is also called by parse_object_or_die(), is thread-unsafe and also called by object reading functions. It's unlikely to really fall into a data race with these operations as the volume of calls to them is usually very low. But we better protect ourselves against this possibility, anyway. So, to solve these issues, move both of these function calls into the critical section of grep_read_mutex. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matheus Tavares committed Jan 15, 2020 at 23:39 UTC faf123c730df1a545e2dc7ca82fd437e93fc519c
1 file changed +3 -4
builtin/grep.c
+3 -4
@@ -407,8 +407,7 @@ static int grep_submodule(struct grep_opt *opt,
407 {
408 struct repository subrepo;
409 struct repository *superproject = opt->repo;
410 - const struct submodule *sub = submodule_from_path(superproject,
411 - &null_oid, path);
410 + const struct submodule *sub;
411 struct grep_opt subopt;
412 int hit;
413
@@ -419,6 +418,7 @@ static int grep_submodule(struct grep_opt *opt,
418 * object.
419 */
420 grep_read_lock();
421 + sub = submodule_from_path(superproject, &null_oid, path);
422
423 if (!is_submodule_active(superproject, path)) {
424 grep_read_unlock();
@@ -455,9 +455,8 @@ static int grep_submodule(struct grep_opt *opt,
455 unsigned long size;
456 struct strbuf base = STRBUF_INIT;
457
458 - object = parse_object_or_die(oid, oid_to_hex(oid));
459 -
458 grep_read_lock();
459 + object = parse_object_or_die(oid, oid_to_hex(oid));
460 data = read_object_with_reference(&subrepo,
461 &object->oid, tree_type,
462 &size, NULL);