merge-ort: fix segmentation fault in read-only repositories
If the blob/tree objects cannot be written, we really need the merge operations to fail, and not to continue (and then try to access the tree object which is however still set to `NULL`). Let's stop ignoring the return value of `write_object_file()` and `write_tree()` and set `clean = -1` in the error case. Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Johannes Schindelin committed
Sep 28, 2022 at 07:29 UTC
0b55d930a69692c7f4e7b90e35fa41f6c46df4bc
2 files changed
+50
-25
merge-ort.c
+41
-25
@@ -3571,15 +3571,15 @@ static int tree_entry_order(const void *a_, const void *b_)
3571
b->string, strlen(b->string), bmi->result.mode);
3572
}
3573
3574
-static void write_tree(struct object_id *result_oid,
3575
- struct string_list *versions,
3576
- unsigned int offset,
3577
- size_t hash_size)
3574
+static int write_tree(struct object_id *result_oid,
3575
+ struct string_list *versions,
3576
+ unsigned int offset,
3577
+ size_t hash_size)
3578
{
3579
size_t maxlen = 0, extra;
3580
unsigned int nr;
3581
struct strbuf buf = STRBUF_INIT;
3582
- int i;
3582
+ int i, ret = 0;
3583
3584
assert(offset <= versions->nr);
3585
nr = versions->nr - offset;
@@ -3605,8 +3605,10 @@ static void write_tree(struct object_id *result_oid,
3605
}
3606
3607
/* Write this object file out, and record in result_oid */
3608
- write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid);
3608
+ if (write_object_file(buf.buf, buf.len, OBJ_TREE, result_oid))
3609
+ ret = -1;
3610
strbuf_release(&buf);
3611
+ return ret;
3612
}
3613
3614
static void record_entry_for_tree(struct directory_versions *dir_metadata,
@@ -3625,13 +3627,13 @@ static void record_entry_for_tree(struct directory_versions *dir_metadata,
3627
basename)->util = &mi->result;
3628
}
3629
3628
-static void write_completed_directory(struct merge_options *opt,
3629
- const char *new_directory_name,
3630
- struct directory_versions *info)
3630
+static int write_completed_directory(struct merge_options *opt,
3631
+ const char *new_directory_name,
3632
+ struct directory_versions *info)
3633
{
3634
const char *prev_dir;
3635
struct merged_info *dir_info = NULL;
3634
- unsigned int offset;
3636
+ unsigned int offset, ret = 0;
3637
3638
/*
3639
* Some explanation of info->versions and info->offsets...
@@ -3717,7 +3719,7 @@ static void write_completed_directory(struct merge_options *opt,
3719
* strcmp here.)
3720
*/
3721
if (new_directory_name == info->last_directory)
3720
- return;
3722
+ return 0;
3723
3724
/*
3725
* If we are just starting (last_directory is NULL), or last_directory
@@ -3739,7 +3741,7 @@ static void write_completed_directory(struct merge_options *opt,
3741
*/
3742
string_list_append(&info->offsets,
3743
info->last_directory)->util = (void*)offset;
3742
- return;
3744
+ return 0;
3745
}
3746
3747
/*
@@ -3769,8 +3771,9 @@ static void write_completed_directory(struct merge_options *opt,
3771
*/
3772
dir_info->is_null = 0;
3773
dir_info->result.mode = S_IFDIR;
3772
- write_tree(&dir_info->result.oid, &info->versions, offset,
3773
- opt->repo->hash_algo->rawsz);
3774
+ if (write_tree(&dir_info->result.oid, &info->versions, offset,
3775
+ opt->repo->hash_algo->rawsz) < 0)
3776
+ ret = -1;
3777
}
3778
3779
/*
@@ -3798,6 +3801,8 @@ static void write_completed_directory(struct merge_options *opt,
3801
/* And, of course, we need to update last_directory to match. */
3802
info->last_directory = new_directory_name;
3803
info->last_directory_len = strlen(info->last_directory);
3804
+
3805
+ return ret;
3806
}
3807
3808
/* Per entry merge function */
@@ -4214,8 +4219,8 @@ static void prefetch_for_content_merges(struct merge_options *opt,
4219
oid_array_clear(&to_fetch);
4220
}
4221
4217
-static void process_entries(struct merge_options *opt,
4218
- struct object_id *result_oid)
4222
+static int process_entries(struct merge_options *opt,
4223
+ struct object_id *result_oid)
4224
{
4225
struct hashmap_iter iter;
4226
struct strmap_entry *e;
@@ -4224,11 +4229,12 @@ static void process_entries(struct merge_options *opt,
4229
struct directory_versions dir_metadata = { STRING_LIST_INIT_NODUP,
4230
STRING_LIST_INIT_NODUP,
4231
NULL, 0 };
4232
+ int ret = 0;
4233
4234
trace2_region_enter("merge", "process_entries setup", opt->repo);
4235
if (strmap_empty(&opt->priv->paths)) {
4236
oidcpy(result_oid, opt->repo->hash_algo->empty_tree);
4231
- return;
4237
+ return 0;
4238
}
4239
4240
/* Hack to pre-allocate plist to the desired size */
@@ -4270,8 +4276,11 @@ static void process_entries(struct merge_options *opt,
4276
*/
4277
struct merged_info *mi = entry->util;
4278
4273
- write_completed_directory(opt, mi->directory_name,
4274
- &dir_metadata);
4279
+ if (write_completed_directory(opt, mi->directory_name,
4280
+ &dir_metadata) < 0) {
4281
+ ret = -1;
4282
+ goto cleanup;
4283
+ }
4284
if (mi->clean)
4285
record_entry_for_tree(&dir_metadata, path, mi);
4286
else {
@@ -4291,12 +4300,16 @@ static void process_entries(struct merge_options *opt,
4300
fflush(stdout);
4301
BUG("dir_metadata accounting completely off; shouldn't happen");
4302
}
4294
- write_tree(result_oid, &dir_metadata.versions, 0,
4295
- opt->repo->hash_algo->rawsz);
4303
+ if (write_tree(result_oid, &dir_metadata.versions, 0,
4304
+ opt->repo->hash_algo->rawsz) < 0)
4305
+ ret = -1;
4306
+cleanup:
4307
string_list_clear(&plist, 0);
4308
string_list_clear(&dir_metadata.versions, 0);
4309
string_list_clear(&dir_metadata.offsets, 0);
4310
trace2_region_leave("merge", "process_entries cleanup", opt->repo);
4311
+
4312
+ return ret;
4313
}
4314
4315
/*** Function Grouping: functions related to merge_switch_to_result() ***/
@@ -4928,15 +4941,18 @@ redo:
4941
}
4942
4943
trace2_region_enter("merge", "process_entries", opt->repo);
4931
- process_entries(opt, &working_tree_oid);
4944
+ if (process_entries(opt, &working_tree_oid) < 0)
4945
+ result->clean = -1;
4946
trace2_region_leave("merge", "process_entries", opt->repo);
4947
4948
/* Set return values */
4949
result->path_messages = &opt->priv->conflicts;
4950
4937
- result->tree = parse_tree_indirect(&working_tree_oid);
4938
- /* existence of conflicted entries implies unclean */
4939
- result->clean &= strmap_empty(&opt->priv->conflicted);
4951
+ if (result->clean >= 0) {
4952
+ result->tree = parse_tree_indirect(&working_tree_oid);
4953
+ /* existence of conflicted entries implies unclean */
4954
+ result->clean &= strmap_empty(&opt->priv->conflicted);
4955
+ }
4956
if (!opt->priv->call_depth) {
4957
result->priv = opt->priv;
4958
result->_properly_initialized = RESULT_INITIALIZED;
t/t4301-merge-tree-write-tree.sh
+9
@@ -810,4 +810,13 @@ test_expect_success 'can override merge of unrelated histories' '
810
test_cmp expect actual
811
'
812
813
+test_expect_success SANITY 'merge-ort fails gracefully in a read-only repository' '
814
+ git init --bare read-only &&
815
+ git push read-only side1 side2 side3 &&
816
+ test_when_finished "chmod -R u+w read-only" &&
817
+ chmod -R a-w read-only &&
818
+ test_must_fail git -C read-only merge-tree side1 side3 &&
819
+ test_must_fail git -C read-only merge-tree side1 side2
820
+'
821
+
822
test_done