Re: [PATCH] reftable/iter: fix undefined behavior in indexed_table_ref_iter_next
This is my first contribution attempt I am so happy I didn’t messed it up I found few more issues I will address them one by one You made my day thank you
Sent from Proton Mail for iOS.
-------- Original Message -------- On Saturday, 01/03/26 at 09:35 Pushkar Singh <pushkarkumarsingh1970@gmail.com> wrote: Hi Tsahi,
Thanks for working on this.
The issue and fix make sense to me. Guarding access to the val2 members behind a value_type check avoids the undefined behavior noted by the existing comment, and explicitly handling REFTABLE_REF_VAL1 here matches the pattern already used in filtering_ref_iterator_next().
I didn’t spot any issues with the control flow or logic in this change.
Thanks for addressing this.
Pushkar
On Sat, Jan 3, 2026 at 12:47 AM Tsahi Elkayam <Tsahi.Elkayam@protonmail.com> wrote:
Show 50 quoted lines
>
>
>
> The indexed_table_ref_iter_next() function accesses ref->value.val2
> without first checking the ref's value_type. This is undefined behavior
> when the ref is not of type REFTABLE_REF_VAL2.
>
> The correct pattern is already used in filtering_ref_iterator_next()
> which checks value_type before accessing the appropriate union member.
> Apply the same pattern here:
>
> - Check for REFTABLE_REF_VAL2 before accessing val2 members
> - Add missing check for REFTABLE_REF_VAL1 to handle single-value refs
>
> This was marked with a "/* BUG */" comment indicating the issue was
> known but not yet fixed.
>
> Signed-off-by: Tsahi Elkayam <Tsahi.Elkayam@protonmail.com>
> ---
> reftable/iter.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/reftable/iter.c b/reftable/iter.c
> index 2ecc52b336..2eee65bb1e 100644
> --- a/reftable/iter.c
> +++ b/reftable/iter.c
> @@ -171,12 +171,15 @@ static int indexed_table_ref_iter_next(void *p, struct reftable_record *rec)
> }
> continue;
> }
> - /* BUG */
> - if (!memcmp(it->oid.buf, ref->value.val2.target_value,
> - it->oid.len) ||
> - !memcmp(it->oid.buf, ref->value.val2.value, it->oid.len)) {
> + if (ref->value_type == REFTABLE_REF_VAL2 &&
> + (!memcmp(it->oid.buf, ref->value.val2.target_value,
> + it->oid.len) ||
> + !memcmp(it->oid.buf, ref->value.val2.value, it->oid.len)))
> + return 0;
> +
> + if (ref->value_type == REFTABLE_REF_VAL1 &&
> + !memcmp(it->oid.buf, ref->value.val1, it->oid.len))
> return 0;
> - }
> }
> }
>
> --
> 2.37.1 (Apple Git-137.1)
>
