diff mbox series

[v4,03/13] Linux: readdir_r needs to report getdents failures (bug 32124)

Message ID 69eb384e8ded80045d1a559186b8c0be1e354b0a.1725047142.git.fweimer@redhat.com
State Committed, archived
Delegated to: DJ Delorie
Headers
Series FUSE-based testing for file system functions |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Florian Weimer Aug. 30, 2024, 7:52 p.m. UTC 
  Upon error, return the errno value set by the __getdents call
in __readdir_unlocked.  Previously, kernel-reported errors
were ignored.
---
 support/support_readdir_check.c     |  3 +-
 support/support_readdir_r_check.c   | 10 ++++-
 support/tst-xdirent.c               | 22 +++++++----
 support/xdirent.h                   | 57 +++++++++++++++++++++++------
 sysdeps/unix/sysv/linux/readdir_r.c | 11 +++++-
 5 files changed, 79 insertions(+), 24 deletions(-)

Comments

DJ Delorie Aug. 31, 2024, 12:40 a.m. UTC | #1 
Florian Weimer <fweimer@redhat.com> writes:
> Upon error, return the errno value set by the __getdents call
> in __readdir_unlocked.  Previously, kernel-reported errors
> were ignored.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>

> diff --git a/support/support_readdir_check.c b/support/support_readdir_check.c
>  void *
> -support_readdir_check (const char *name, void *result)
> +support_readdir_check (const char *name, void *result, int saved_errno)
>  {
>    if (result == NULL && errno != 0)
>      FAIL_EXIT1 ("%s: %m", name);
> +  errno = saved_errno;
>    return result;
>  }

Ok.

> diff --git a/support/support_readdir_r_check.c b/support/support_readdir_r_check.c
> index 8675b08ff3..6bbb0d0b32 100644
> --- a/support/support_readdir_r_check.c
> +++ b/support/support_readdir_r_check.c
> @@ -21,9 +21,15 @@
>  #include <support/check.h>
>  
>  int
> -support_readdir_r_check (const char *name, int result)
> +support_readdir_r_check (const char *name, int result, void *buf, void *ptr)
>  {
>    if (result != 0)
> -    FAIL_EXIT1 ("%s: %m", name);
> +    {
> +      errno = result;
> +      FAIL_EXIT1 ("%s: %m", name);
> +    }
> +  if (buf != ptr)
> +    FAIL_EXIT1 ("%s: buffer pointer and returned pointer differ: %p != %p",
> +                name, buf, ptr);
>    return result;
>  }

Ok.

> diff --git a/support/tst-xdirent.c b/support/tst-xdirent.c
> index 7012b3185b..642483165a 100644
> --- a/support/tst-xdirent.c
> +++ b/support/tst-xdirent.c
> @@ -30,6 +30,8 @@ do_test (void)
>      struct dirent *e = xreaddir (d);
>      /* Assume that the "." special entry always comes first.  */
>      TEST_COMPARE_STRING (e->d_name, ".");
> +    while (xreaddir (d) != NULL)
> +      ;
>      xclosedir (d);
>    }

Ok.

> @@ -37,6 +39,8 @@ do_test (void)
>      DIR *d = xopendir (".");
>      struct dirent64 *e = xreaddir64 (d);
>      TEST_COMPARE_STRING (e->d_name, ".");
> +    while (xreaddir64 (d) != NULL)
> +      ;
>      xclosedir (d);
>    }

Ok.

> @@ -46,19 +50,21 @@ do_test (void)
>  
>    {
>      DIR *d = xopendir (".");
> -    struct dirent buf;
> -    struct dirent *e;
> -    TEST_COMPARE (xreaddir_r (d, &buf, &e), 0);
> -    TEST_COMPARE_STRING (e->d_name, ".");
> +    struct dirent buf = { 0, };
> +    TEST_VERIFY (xreaddir_r (d, &buf));
> +    TEST_COMPARE_STRING (buf.d_name, ".");
> +    while (xreaddir_r (d, &buf))
> +      ;
>      xclosedir (d);
>    }

Ok.

>    {
>      DIR *d = xopendir (".");
> -    struct dirent64 buf;
> -    struct dirent64 *e;
> -    TEST_COMPARE (xreaddir64_r (d, &buf, &e), 0);
> -    TEST_COMPARE_STRING (e->d_name, ".");
> +    struct dirent64 buf = { 0, };
> +    TEST_VERIFY (xreaddir64_r (d, &buf));
> +    TEST_COMPARE_STRING (buf.d_name, ".");
> +    while (xreaddir64_r (d, &buf))
> +      ;
>      xclosedir (d);
>    }

Ok.

> diff --git a/support/xdirent.h b/support/xdirent.h
> index 83f41dba6b..8465d70ec1 100644
> --- a/support/xdirent.h
> +++ b/support/xdirent.h
> @@ -22,6 +22,8 @@
>  #include <dirent.h>
>  #include <errno.h>
>  #include <libc-diag.h>
> +#include <stdbool.h>
> +#include <stddef.h>

Ok.

>  
> -void *support_readdir_check (const char *, void *);
> -#define xreaddir(d)                                                     \
> -  ((struct dirent *) support_readdir_check ("readdir",                  \
> -                                            (errno = 0, readdir ((d)))))
> -#define xreaddir64(d)                                                   \
> -  ((struct dirent64 *) support_readdir_check ("readdir64",              \
> -                                              (errno = 0, readdir64 ((d)))))
> +void *support_readdir_check (const char *, void *, int);
>  
> -int support_readdir_r_check (const char *, int);
> +static __attribute__ ((unused)) struct dirent *
> +xreaddir (DIR *stream)
> +{
> +  int saved_errno = errno;
> +  errno = 0;
> +  struct dirent *result = readdir (stream);
> +  return support_readdir_check ("readdir", result, saved_errno);
> +}
> +
> +static __attribute__ ((unused)) struct dirent64 *
> +xreaddir64 (DIR *stream)
> +{
> +  int saved_errno = errno;
> +  errno = 0;
> +  struct dirent64 *result = readdir64 (stream);
> +  return support_readdir_check ("readdir64", result, saved_errno);
> +}

ok

>  /* The functions readdir_r, readdir64_r were deprecated in glibc 2.24.  */
>  DIAG_PUSH_NEEDS_COMMENT;
>  DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
> -#define xreaddir_r(d, e, r)                                             \
> -  (support_readdir_r_check ("readdir_r", readdir_r ((d), (e), (r))))
> -#define xreaddir64_r(d, e, r) \
> -  (support_readdir_r_check ("readdir64_r", readdir64_r ((d), (e), (r))))
> +
> +int support_readdir_r_check (const char *, int, void *, void *);
> +
> +static __attribute__ ((unused)) bool
> +xreaddir_r (DIR *stream, struct dirent *buf)
> +{
> +  struct dirent *ptr;
> +  int ret = readdir_r (stream, buf, &ptr);
> +  if (ret == 0 && ptr == NULL)
> +    return false;
> +  support_readdir_r_check ("readdir_r", ret, buf, ptr);
> +  return true;
> +}
> +
> +static __attribute__ ((unused)) bool
> +xreaddir64_r (DIR *stream, struct dirent64 *buf)
> +{
> +  struct dirent64 *ptr;
> +  int ret = readdir64_r (stream, buf, &ptr);
> +  if (ret == 0 && ptr == NULL)
> +    return false;
> +  support_readdir_r_check ("readdir64_r", ret, buf, ptr);
> +  return true;
> +}
> +
>  DIAG_POP_NEEDS_COMMENT;

Ok.

> diff --git a/sysdeps/unix/sysv/linux/readdir_r.c b/sysdeps/unix/sysv/linux/readdir_r.c
>  {
>    struct dirent *dp;
>    size_t reclen;
> +  int saved_errno = errno;
>  
>    __libc_lock_lock (dirp->lock);
>  
>    while (1)
>      {
> +      /* If errno is changed from 0, the NULL return value indicates
> +	 an actual error.  It overrides a pending ENAMETOOLONG error.  */
> +      __set_errno (0);
>        dp = __readdir_unlocked (dirp);
>        if (dp == NULL)
> -	break;
> +	{
> +	  if (errno != 0)
> +	    dirp->errcode = errno;
> +	  break;
> +	}
>  
>        reclen = dp->d_reclen;
>        if (reclen <= offsetof (struct dirent, d_name) + NAME_MAX + 1)
> @@ -61,6 +69,7 @@ __readdir_r (DIR *dirp, struct dirent *entry, struct dirent **result)
>  
>    __libc_lock_unlock (dirp->lock);
>  
> +  __set_errno (saved_errno);
>    return dp != NULL ? 0 : dirp->errcode;
>  }

Ok.
diff mbox series

Patch

diff --git a/support/support_readdir_check.c b/support/support_readdir_check.c
index 8d48a4c5a4..5687004276 100644
--- a/support/support_readdir_check.c
+++ b/support/support_readdir_check.c
@@ -21,9 +21,10 @@ 
 #include <support/check.h>
 
 void *
-support_readdir_check (const char *name, void *result)
+support_readdir_check (const char *name, void *result, int saved_errno)
 {
   if (result == NULL && errno != 0)
     FAIL_EXIT1 ("%s: %m", name);
+  errno = saved_errno;
   return result;
 }
diff --git a/support/support_readdir_r_check.c b/support/support_readdir_r_check.c
index 8675b08ff3..6bbb0d0b32 100644
--- a/support/support_readdir_r_check.c
+++ b/support/support_readdir_r_check.c
@@ -21,9 +21,15 @@ 
 #include <support/check.h>
 
 int
-support_readdir_r_check (const char *name, int result)
+support_readdir_r_check (const char *name, int result, void *buf, void *ptr)
 {
   if (result != 0)
-    FAIL_EXIT1 ("%s: %m", name);
+    {
+      errno = result;
+      FAIL_EXIT1 ("%s: %m", name);
+    }
+  if (buf != ptr)
+    FAIL_EXIT1 ("%s: buffer pointer and returned pointer differ: %p != %p",
+                name, buf, ptr);
   return result;
 }
diff --git a/support/tst-xdirent.c b/support/tst-xdirent.c
index 7012b3185b..642483165a 100644
--- a/support/tst-xdirent.c
+++ b/support/tst-xdirent.c
@@ -30,6 +30,8 @@  do_test (void)
     struct dirent *e = xreaddir (d);
     /* Assume that the "." special entry always comes first.  */
     TEST_COMPARE_STRING (e->d_name, ".");
+    while (xreaddir (d) != NULL)
+      ;
     xclosedir (d);
   }
 
@@ -37,6 +39,8 @@  do_test (void)
     DIR *d = xopendir (".");
     struct dirent64 *e = xreaddir64 (d);
     TEST_COMPARE_STRING (e->d_name, ".");
+    while (xreaddir64 (d) != NULL)
+      ;
     xclosedir (d);
   }
 
@@ -46,19 +50,21 @@  do_test (void)
 
   {
     DIR *d = xopendir (".");
-    struct dirent buf;
-    struct dirent *e;
-    TEST_COMPARE (xreaddir_r (d, &buf, &e), 0);
-    TEST_COMPARE_STRING (e->d_name, ".");
+    struct dirent buf = { 0, };
+    TEST_VERIFY (xreaddir_r (d, &buf));
+    TEST_COMPARE_STRING (buf.d_name, ".");
+    while (xreaddir_r (d, &buf))
+      ;
     xclosedir (d);
   }
 
   {
     DIR *d = xopendir (".");
-    struct dirent64 buf;
-    struct dirent64 *e;
-    TEST_COMPARE (xreaddir64_r (d, &buf, &e), 0);
-    TEST_COMPARE_STRING (e->d_name, ".");
+    struct dirent64 buf = { 0, };
+    TEST_VERIFY (xreaddir64_r (d, &buf));
+    TEST_COMPARE_STRING (buf.d_name, ".");
+    while (xreaddir64_r (d, &buf))
+      ;
     xclosedir (d);
   }
 
diff --git a/support/xdirent.h b/support/xdirent.h
index 83f41dba6b..8465d70ec1 100644
--- a/support/xdirent.h
+++ b/support/xdirent.h
@@ -22,6 +22,8 @@ 
 #include <dirent.h>
 #include <errno.h>
 #include <libc-diag.h>
+#include <stdbool.h>
+#include <stddef.h>
 
 __BEGIN_DECLS
 
@@ -29,23 +31,54 @@  DIR *xopendir (const char *path);
 DIR *xfdopendir (int fd);
 void xclosedir (DIR *);
 
-void *support_readdir_check (const char *, void *);
-#define xreaddir(d)                                                     \
-  ((struct dirent *) support_readdir_check ("readdir",                  \
-                                            (errno = 0, readdir ((d)))))
-#define xreaddir64(d)                                                   \
-  ((struct dirent64 *) support_readdir_check ("readdir64",              \
-                                              (errno = 0, readdir64 ((d)))))
+void *support_readdir_check (const char *, void *, int);
 
-int support_readdir_r_check (const char *, int);
+static __attribute__ ((unused)) struct dirent *
+xreaddir (DIR *stream)
+{
+  int saved_errno = errno;
+  errno = 0;
+  struct dirent *result = readdir (stream);
+  return support_readdir_check ("readdir", result, saved_errno);
+}
+
+static __attribute__ ((unused)) struct dirent64 *
+xreaddir64 (DIR *stream)
+{
+  int saved_errno = errno;
+  errno = 0;
+  struct dirent64 *result = readdir64 (stream);
+  return support_readdir_check ("readdir64", result, saved_errno);
+}
 
 /* The functions readdir_r, readdir64_r were deprecated in glibc 2.24.  */
 DIAG_PUSH_NEEDS_COMMENT;
 DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wdeprecated-declarations");
-#define xreaddir_r(d, e, r)                                             \
-  (support_readdir_r_check ("readdir_r", readdir_r ((d), (e), (r))))
-#define xreaddir64_r(d, e, r) \
-  (support_readdir_r_check ("readdir64_r", readdir64_r ((d), (e), (r))))
+
+int support_readdir_r_check (const char *, int, void *, void *);
+
+static __attribute__ ((unused)) bool
+xreaddir_r (DIR *stream, struct dirent *buf)
+{
+  struct dirent *ptr;
+  int ret = readdir_r (stream, buf, &ptr);
+  if (ret == 0 && ptr == NULL)
+    return false;
+  support_readdir_r_check ("readdir_r", ret, buf, ptr);
+  return true;
+}
+
+static __attribute__ ((unused)) bool
+xreaddir64_r (DIR *stream, struct dirent64 *buf)
+{
+  struct dirent64 *ptr;
+  int ret = readdir64_r (stream, buf, &ptr);
+  if (ret == 0 && ptr == NULL)
+    return false;
+  support_readdir_r_check ("readdir64_r", ret, buf, ptr);
+  return true;
+}
+
 DIAG_POP_NEEDS_COMMENT;
 
 __END_DECLS
diff --git a/sysdeps/unix/sysv/linux/readdir_r.c b/sysdeps/unix/sysv/linux/readdir_r.c
index ffd5262cf5..1d595688f7 100644
--- a/sysdeps/unix/sysv/linux/readdir_r.c
+++ b/sysdeps/unix/sysv/linux/readdir_r.c
@@ -25,14 +25,22 @@  __readdir_r (DIR *dirp, struct dirent *entry, struct dirent **result)
 {
   struct dirent *dp;
   size_t reclen;
+  int saved_errno = errno;
 
   __libc_lock_lock (dirp->lock);
 
   while (1)
     {
+      /* If errno is changed from 0, the NULL return value indicates
+	 an actual error.  It overrides a pending ENAMETOOLONG error.  */
+      __set_errno (0);
       dp = __readdir_unlocked (dirp);
       if (dp == NULL)
-	break;
+	{
+	  if (errno != 0)
+	    dirp->errcode = errno;
+	  break;
+	}
 
       reclen = dp->d_reclen;
       if (reclen <= offsetof (struct dirent, d_name) + NAME_MAX + 1)
@@ -61,6 +69,7 @@  __readdir_r (DIR *dirp, struct dirent *entry, struct dirent **result)
 
   __libc_lock_unlock (dirp->lock);
 
+  __set_errno (saved_errno);
   return dp != NULL ? 0 : dirp->errcode;
 }