ref-cache: fix invalid free operation in `free_ref_entry`

In cfd971520e (refs: keep track of unresolved reference value in
iterators, 2024-08-09), we added a new field "referent" into the "struct
ref" structure. In order to free the "referent", we unconditionally
freed the "referent" by simply adding a "free" statement.

However, this is a bad usage. Because when ref entry is either directory
or loose ref, we will always execute the following statement:

  free(entry->u.value.referent);

This does not make sense. We should never access the "entry->u.value"
field when "entry" is a directory. However, the change obviously doesn't
break the tests. Let's analysis why.

The anonymous union in the "ref_entry" has two members: one is "struct
ref_value", another is "struct ref_dir". On a 64-bit machine, the size
of "struct ref_dir" is 32 bytes, which is smaller than the 48-byte size
of "struct ref_value". And the offset of "referent" field in "struct
ref_value" is 40 bytes. So, whenever we create a new "ref_entry" for a
directory, we will leave the offset from 40 bytes to 48 bytes untouched,
which means the value for this memory is zero (NULL). It's OK to free a
NULL pointer, but this is merely a coincidence of memory layout.

To fix this issue, we now ensure that "free(entry->u.value.referent)" is
only called when "entry->flag" indicates that it represents a loose
reference and not a directory to avoid the invalid memory operation.

Signed-off-by: shejialuo <shejialuo@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/refs/ref-cache.c b/refs/ref-cache.c
index 35bae7e05dedcdbdadbc352ac9e749dc1fc434ff..02f09e4df88f23955678fa5cee4ef1ac4832cf2d 100644 (file)
--- a/refs/ref-cache.c
+++ b/refs/ref-cache.c
@@ -68,8 +68,9 @@ static void free_ref_entry(struct ref_entry *entry)
                 * trigger the reading of loose refs.
                 */
                clear_ref_dir(&entry->u.subdir);
+       } else {
+               free(entry->u.value.referent);
        }
-       free(entry->u.value.referent);
        free(entry);
 }