Prefer readlink to lstat+S_ISLNK when easy

To test for a symlink, use readlink, not lstat+S_ISLNK,
when the lstat is used only for the symlink test.
This avoids EOVERFLOW issues.
* lib/lchown.c (rpl_lchown) [CHOWN_CHANGE_TIME_BUG]:
* lib/rename.c (rpl_rename):
[!(_WIN32 && !__CYGWIN__) && (RENAME_TRAILING_SLASH_SOURCE_BUG
|| RENAME_DEST_EXISTS_BUG || RENAME_HARD_LINK_BUG)]:
* lib/renameatu.c (renameatu):
[HAVE_RENAMEAT && RENAME_TRAILING_SLASH_SOURCE_BUG]:
* lib/unlink.c (rpl_unlink):
* lib/unlinkat.c (rpl_unlinkat):
* lib/utimens.c (lutimens) [!HAVE_LUTIMENS]:
Prefer readlink to lstat+S_ISLNK.
* modules/lchown, modules/rename, modules/unlink, modules/utimens:
(Depends-on): Add readlink.
* modules/unlinkat (Depends-on): Add fstatat, readlinkat.

diff --git a/ChangeLog b/ChangeLog
index 0a44bca695656586fb9c92df7cf1b898ac36b29f..0cabe966237f88a8121cf131b16ec63b51903a33 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,23 @@
+2025-08-11  Paul Eggert  <eggert@cs.ucla.edu>
+
+       Prefer readlink to lstat+S_ISLNK when easy
+       To test for a symlink, use readlink, not lstat+S_ISLNK,
+       when the lstat is used only for the symlink test.
+       This avoids EOVERFLOW issues.
+       * lib/lchown.c (rpl_lchown) [CHOWN_CHANGE_TIME_BUG]:
+       * lib/rename.c (rpl_rename):
+       [!(_WIN32 && !__CYGWIN__) && (RENAME_TRAILING_SLASH_SOURCE_BUG
+       || RENAME_DEST_EXISTS_BUG || RENAME_HARD_LINK_BUG)]:
+       * lib/renameatu.c (renameatu):
+       [HAVE_RENAMEAT && RENAME_TRAILING_SLASH_SOURCE_BUG]:
+       * lib/unlink.c (rpl_unlink):
+       * lib/unlinkat.c (rpl_unlinkat):
+       * lib/utimens.c (lutimens) [!HAVE_LUTIMENS]:
+       Prefer readlink to lstat+S_ISLNK.
+       * modules/lchown, modules/rename, modules/unlink, modules/utimens:
+       (Depends-on): Add readlink.
+       * modules/unlinkat (Depends-on): Add fstatat, readlinkat.
+
 2025-08-11  Bruno Haible  <bruno@clisp.org>
 
        nlcanon tests: Fix test failure on Solaris.

diff --git a/lib/lchown.c b/lib/lchown.c
index 74cb9afa411c01c289eaf901036d33db97f06770..ce7d31730a417c30f85f47bdfac4dabece6d10c5 100644 (file)
--- a/lib/lchown.c
+++ b/lib/lchown.c
@@ -77,9 +77,19 @@ rpl_lchown (const char *file, uid_t uid, gid_t gid)
 
   if (gid != (gid_t) -1 || uid != (uid_t) -1)
     {
-      if (lstat (file, &st))
-        return -1;
+      /* Prefer readlink to lstat+S_ISLNK, to avoid EOVERFLOW issues
+         in the common case where FILE is a non-symlink.  */
+      char linkbuf[1];
+      int r = readlink (file, linkbuf, 1);
+      if (r < 0)
+        return errno == EINVAL ? chown (file, uid, gid) : r;
+
+      /* Later code can use the status, so get it if possible.  */
+      r = lstat (file, &st);
+      if (r < 0)
+        return r;
       stat_valid = true;
+      /* An easy check: did FILE change from a symlink to a non-symlink?  */
       if (!S_ISLNK (st.st_mode))
         return chown (file, uid, gid);
     }

diff --git a/lib/rename.c b/lib/rename.c
index 4e524c0ab905397ae16166419fd499c3c154a064..f4f191d61f9d2fbefa7bb6f35ae8bcb3ef30b1a5 100644 (file)
--- a/lib/rename.c
+++ b/lib/rename.c
@@ -389,13 +389,14 @@ rpl_rename (char const *src, char const *dst)
           goto out;
         }
       strip_trailing_slashes (src_temp);
-      if (lstat (src_temp, &src_st))
+      char linkbuf[1];
+      if (0 <= readlink (src_temp, linkbuf, 1))
+        goto out;
+      if (errno != EINVAL)
         {
           rename_errno = errno;
           goto out;
         }
-      if (S_ISLNK (src_st.st_mode))
-        goto out;
     }
   if (dst_slash)
     {
@@ -406,16 +407,14 @@ rpl_rename (char const *src, char const *dst)
           goto out;
         }
       strip_trailing_slashes (dst_temp);
-      if (lstat (dst_temp, &dst_st))
+      char linkbuf[1];
+      if (0 <= readlink (dst_temp, linkbuf, 1))
+        goto out;
+      if (errno != EINVAL && errno != ENOENT)
         {
-          if (errno != ENOENT)
-            {
-              rename_errno = errno;
-              goto out;
-            }
+          rename_errno = errno;
+          goto out;
         }
-      else if (S_ISLNK (dst_st.st_mode))
-        goto out;
     }
 # endif /* RENAME_TRAILING_SLASH_SOURCE_BUG || RENAME_DEST_EXISTS_BUG
            || RENAME_HARD_LINK_BUG */

diff --git a/lib/renameatu.c b/lib/renameatu.c
index b71e8e3fe425df633f0012d5a66124f33887e267..725e031abce8890744f0cd97eed67f4032dbf5e5 100644 (file)
--- a/lib/renameatu.c
+++ b/lib/renameatu.c
@@ -204,12 +204,16 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
           goto out;
         }
       strip_trailing_slashes (src_temp);
-      if (fstatat (fd1, src_temp, &src_st, AT_SYMLINK_NOFOLLOW))
+      char linkbuf[1];
+      if (readlinkat (fd1, src_temp, linkbuf, sizeof linkbuf) < 0)
         {
-          rename_errno = errno;
-          goto out;
+          if (errno != EINVAL)
+            {
+              rename_errno = errno;
+              goto out;
+            }
         }
-      if (S_ISLNK (src_st.st_mode))
+      else
         goto out;
     }
   if (dst_slash)
@@ -221,8 +225,8 @@ renameatu (int fd1, char const *src, int fd2, char const *dst,
           goto out;
         }
       strip_trailing_slashes (dst_temp);
-      char readlink_buf[1];
-      if (readlinkat (fd2, dst_temp, readlink_buf, sizeof readlink_buf) < 0)
+      char linkbuf[1];
+      if (readlinkat (fd2, dst_temp, linkbuf, sizeof linkbuf) < 0)
         {
           if (errno != ENOENT && errno != EINVAL)
             {

diff --git a/lib/unlink.c b/lib/unlink.c
index d2454ca624db4377d453a7f2806194e01d04ba0d..6459e22e0b5a6a18729a8c289c0a65efbb98127d 100644 (file)
--- a/lib/unlink.c
+++ b/lib/unlink.c
@@ -57,7 +57,7 @@ rpl_unlink (char const *name)
          symlink instead of the directory.  Technically, we could use
          realpath to find the canonical directory name to attempt
          deletion on.  But that is a lot of work for a corner case; so
-         we instead just use an lstat on the shortened name, and
+         we instead just use a readlink on the shortened name, and
          reject symlinks with trailing slashes.  The root user of
          unlink(1) will just have to live with the rule that they
          can't delete a directory via a symlink.  */
@@ -72,7 +72,9 @@ rpl_unlink (char const *name)
           memcpy (short_name, name, len);
           while (len && ISSLASH (short_name[len - 1]))
             short_name[--len] = '\0';
-          if (len && (lstat (short_name, &st) || S_ISLNK (st.st_mode)))
+          char linkbuf[1];
+          if (len && ! (readlink (short_name, linkbuf, 1) < 0
+                        && errno == EINVAL))
             {
               free (short_name);
               errno = EPERM;

diff --git a/lib/unlinkat.c b/lib/unlinkat.c
index 7fb037199609aac16bc19b86a9aad7da68650aad..6873ba558da548de58e63c7350e39ea2cbf454cc 100644 (file)
--- a/lib/unlinkat.c
+++ b/lib/unlinkat.c
@@ -71,8 +71,8 @@ rpl_unlinkat (int fd, char const *name, int flag)
           memcpy (short_name, name, len);
           while (len && ISSLASH (short_name[len - 1]))
             short_name[--len] = '\0';
-          if (len && (fstatat (fd, short_name, &st, AT_SYMLINK_NOFOLLOW)
-                      || S_ISLNK (st.st_mode)))
+          if (len && (readlinkat (fd, short_name, linkbuf, 1) < 0
+                      || errno == EINVAL))
             {
               free (short_name);
               errno = EPERM;

diff --git a/lib/utimens.c b/lib/utimens.c
index 28e4295f02525f680584d5776a7672fdc1b34234..4122e4ff2239029f06565f3609c09ed920c423da 100644 (file)
--- a/lib/utimens.c
+++ b/lib/utimens.c
@@ -670,9 +670,17 @@ lutimens (char const *file, struct timespec const timespec[2])
 # endif /* HAVE_LUTIMES && !HAVE_UTIMENSAT */
 
   /* Out of luck for symlinks, but we still handle regular files.  */
-  if (!(adjustment_needed || REPLACE_FUNC_STAT_FILE) && lstat (file, &st))
-    return -1;
-  if (!S_ISLNK (st.st_mode))
+  bool not_symlink;
+  if (adjustment_needed || REPLACE_FUNC_STAT_FILE)
+    not_symlink = !S_ISLNK (st.st_mode);
+  else
+    {
+      char linkbuf[1];
+      not_symlink = readlink (file, linkbuf, 1) < 0;
+      if (not_symlink && errno != EINVAL)
+        return -1;
+    }
+  if (not_symlink)
     return fdutimens (-1, file, ts);
   errno = ENOSYS;
   return -1;

diff --git a/modules/lchown b/modules/lchown
index bc05ea47c276bfed5e73edb545e6594cb631424c..45f16d91aa9111f032b5b42a20c682be35722bf2 100644 (file)
--- a/modules/lchown
+++ b/modules/lchown
@@ -7,7 +7,7 @@ m4/lchown.m4
 
 Depends-on:
 unistd-h
-readlink        [test $HAVE_LCHOWN = 0]
+readlink        [test $HAVE_LCHOWN = 0 || test $REPLACE_LCHOWN = 1]
 chown           [test $HAVE_LCHOWN = 0 || test $REPLACE_LCHOWN = 1]
 errno-h         [test $HAVE_LCHOWN = 0 || test $REPLACE_LCHOWN = 1]
 bool            [test $HAVE_LCHOWN = 0 || test $REPLACE_LCHOWN = 1]

diff --git a/modules/rename b/modules/rename
index c0656bd34121e752a994eeabef668324e64bf844..fbc787d3e57c8e47c7512a7a6e76003d3b37ae8c 100644 (file)
--- a/modules/rename
+++ b/modules/rename
@@ -12,6 +12,7 @@ chdir             [test $REPLACE_RENAME = 1]
 dirname-lgpl      [test $REPLACE_RENAME = 1]
 free-posix        [test $REPLACE_RENAME = 1]
 lstat             [test $REPLACE_RENAME = 1]
+readlink          [test $REPLACE_RENAME = 1]
 rmdir             [test $REPLACE_RENAME = 1]
 same-inode        [test $REPLACE_RENAME = 1]
 stat              [test $REPLACE_RENAME = 1]

diff --git a/modules/unlink b/modules/unlink
index d9612b438fbb4e5ed44fb4e14173eb641153ecfc..43c4f8440b5e5b5a95464ac642609cc5f27f61be 100644 (file)
--- a/modules/unlink
+++ b/modules/unlink
@@ -10,6 +10,7 @@ unistd-h
 filename        [test $REPLACE_UNLINK = 1]
 lstat           [test $REPLACE_UNLINK = 1]
 malloc-posix    [test $REPLACE_UNLINK = 1]
+readlink        [test $REPLACE_UNLINK = 1]
 
 configure.ac:
 gl_FUNC_UNLINK

diff --git a/modules/unlinkat b/modules/unlinkat
index 2e914d2fefbbd0f752b5d19d0bb4aed2319175da..aaacfd936a530c399aeee1ebca08ff0589b5d4d4 100644 (file)
--- a/modules/unlinkat
+++ b/modules/unlinkat
@@ -21,6 +21,8 @@ openat-die      [test $HAVE_UNLINKAT = 0]
 rmdir           [test $HAVE_UNLINKAT = 0]
 save-cwd        [test $HAVE_UNLINKAT = 0]
 unlink          [test $HAVE_UNLINKAT = 0]
+fstatat         [test $REPLACE_UNLINKAT = 1]
+readlinkat      [test $REPLACE_UNLINKAT = 1]
 
 configure.ac:
 gl_FUNC_UNLINKAT

diff --git a/modules/utimens b/modules/utimens
index deeecc5bb251a767ca4bcdb0f5dfcf1a7e07754e..aea6a47e665db0df3de05d704049bc3dc2e835dd 100644 (file)
--- a/modules/utimens
+++ b/modules/utimens
@@ -15,6 +15,7 @@ fstat
 lstat
 gettime
 msvc-nothrow
+readlink
 stat
 stat-time
 bool