[BUG] [PATCH] infinite loop due to broken symlink

From: Petr Stodulka <pstodulk@redhat.com>
Date: Mar 23, 2015, 16:04 UTC
Message-ID: <5510399D.4020007@redhat.com>

Hi guys, git goes into an infinite loop due to broken symlink (minimal reproducer [0]). Affected code is in function "resolve_ref_unsafe" in file refs.c - notice 'stat_ref'. There is comment about problem with race condition, hovewer in that case it's regular broken symlink which cause infinite loop. Possible patch could be something like this:

-------------------------------------------------------

diff --git a/refs.c b/refs.c
index e23542b..9efe8d2 100644
--- a/refs.c
+++ b/refs.c
@@ -1356,6 +1356,7 @@ static struct ref_dir *get_loose_refs(struct 
ref_cache *refs)
  /* We allow "recursive" symbolic refs. Only within reason, though */
  #define MAXDEPTH 5
  #define MAXREFLEN (1024)
+#define MAXLOOP 1024

  /*
   * Called by resolve_gitlink_ref_recursive() after it failed to read
@@ -1482,6 +1483,7 @@ const char *resolve_ref_unsafe(const char 
*refname, int resolve_flags, unsigned
         char buffer[256];
         static char refname_buffer[256];
         int bad_name = 0;
+    int loop_counter = 0;

         if (flags)
                 *flags = 0;
@@ -1546,7 +1548,8 @@ const char *resolve_ref_unsafe(const char 
*refname, int resolve_flags, unsigned
                 if (S_ISLNK(st.st_mode)) {
                         len = readlink(path, buffer, sizeof(buffer)-1);
                         if (len < 0) {
-                               if (errno == ENOENT || errno == EINVAL)
+                               if (loop_counter++ < MAXLOOP &&
+                    (errno == ENOENT || errno == EINVAL))
                                         /* inconsistent with lstat; 
retry */
                                         goto stat_ref;
                                 else
@@ -1579,7 +1582,7 @@ const char *resolve_ref_unsafe(const char 
*refname, int resolve_flags, unsigned
                  */
                 fd = open(path, O_RDONLY);
                 if (fd < 0) {
-                       if (errno == ENOENT)
+                       if (loop_counter++ < MAXLOOP && errno == ENOENT)
                                 /* inconsistent with lstat; retry */
                                 goto stat_ref;
                         else
-------------------------------------------------------

If I understand well that simple check of broken symlink is not possible
due to race conditions.

Regards,
Petr

[0] https://bugzilla.redhat.com/show_bug.cgi?id=1204193