When migrating reflogs between reference backends, maintaining the
original order of the reflog entries is crucial. To achieve this, an
`index` field is stored within the `ref_update` struct.
In the reftable backend, before writing any references, the writer must
be configured with the minimum and maximum update index values. The
`max_update_index` is derived from the maximum `ref_update.index` value
in a transaction . The commit bc67b4ab5f (reftable: write correct
max_update_index to header, 2025-01-15) addressed this by propagating the
`max_update_index` value from the transaction to
`write_transaction_table_arg` and, ultimately, to
`reftable_writer_set_limits()`, which sets the min and max index for the
reftable writer.
However, that commit introduced an issue:
- In `reftable_transaction_data`, which contains an array of
`write_transaction_table_arg`, only the first element was assigned the
`max_index` value.
As a result, any elements beyond the first in the array contained
uninitialized `max_index`. The writer contains multiple elements of
`write_transaction_table_arg` to correspond to different worktrees being
written. This uninitialized value was later used to set the
`max_update_index` for the writer, potentially causing overflow or
undefined behavior.
Fix this by:
- Initializing the `max_index` field to 0.
- Moving the assignment of `max_index` in
`reftable_be_transaction_finish()` inside the loop, ensuring all
elements of the array are correctly initialized.
Initializing `max_index` to `0` is not strictly necessary, as all
elements of `write_transaction_table_arg.max_index` are now assigned
correctly. However, this initialization is added for consistency and to
safeguard against potential future changes that might inadvertently
introduce uninitialized memory access.
Reported-by: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
---
Hello,
As suggested, I've redone my patch to make this a relative patch on top of
'kn/reflog-migration-fix'.
This is based on top of maint with 'kn/reflog-migration-fix' merged in.
Thanks
---
refs/reftable-backend.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 289496058e..d39a14c5a4 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1020,6 +1020,7 @@ static int prepare_transaction_update(struct write_transaction_table_arg **out,
arg->updates_nr = 0;
arg->updates_alloc = 0;
arg->updates_expected = 0;
+ arg->max_index = 0;
}
arg->updates_expected++;
@@ -1628,10 +1629,9 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,
struct reftable_transaction_data *tx_data = transaction->backend_data;
int ret = 0;
- if (tx_data->args)
- tx_data->args->max_index = transaction->max_index;
-
for (size_t i = 0; i < tx_data->args_nr; i++) {
+ tx_data->args[i].max_index = transaction->max_index;
+
ret = reftable_addition_add(tx_data->args[i].addition,
write_transaction_table, &tx_data->args[i]);
if (ret < 0)
--
2.47.0
