[PATCH v4 10/13] Linux: Use readdir64_r for compat __old_readdir64_r (bug 32128)
DJ Delorie
dj@redhat.com
Fri Sep 6 20:33:34 GMT 2024
Typo in comment but otherwise LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
Florian Weimer <fweimer@redhat.com> writes:
> It is not necessary to do the conversion at the getdents64
> layer for readdir64_r. Doing int piecewise for readdir64
Typo? s/int/it/
> is slightly simpler and allows deleting __old_getdents64.
>
> This fixes bug 32128 because readdir64_r handles the length
> check correctly.
> ---
> sysdeps/unix/sysv/linux/getdents64.c | 97 --------------------------
> sysdeps/unix/sysv/linux/olddirent.h | 2 -
> sysdeps/unix/sysv/linux/readdir64.c | 90 +++++++++++++-----------
> sysdeps/unix/sysv/linux/readdir64_r.c | 98 ++++++---------------------
> 4 files changed, 71 insertions(+), 216 deletions(-)
>
> diff --git a/sysdeps/unix/sysv/linux/getdents64.c b/sysdeps/unix/sysv/linux/getdents64.c
> index 227fbf21ae..795bd935f0 100644
> --- a/sysdeps/unix/sysv/linux/getdents64.c
> +++ b/sysdeps/unix/sysv/linux/getdents64.c
> @@ -33,100 +33,3 @@ __getdents64 (int fd, void *buf, size_t nbytes)
> }
> libc_hidden_def (__getdents64)
> weak_alias (__getdents64, getdents64)
> -
> -#if _DIRENT_MATCHES_DIRENT64
. . .
> -#endif /* _DIRENT_MATCHES_DIRENT64 */
Ok.
> diff --git a/sysdeps/unix/sysv/linux/olddirent.h b/sysdeps/unix/sysv/linux/olddirent.h
> index 239f790648..065ca41a6e 100644
> --- a/sysdeps/unix/sysv/linux/olddirent.h
> +++ b/sysdeps/unix/sysv/linux/olddirent.h
> @@ -34,8 +34,6 @@ extern struct __old_dirent64 *__old_readdir64 (DIR *__dirp);
> libc_hidden_proto (__old_readdir64);
> extern int __old_readdir64_r (DIR *__dirp, struct __old_dirent64 *__entry,
> struct __old_dirent64 **__result);
> -extern __ssize_t __old_getdents64 (int __fd, char *__buf, size_t __nbytes)
> - attribute_hidden;
> int __old_scandir64 (const char * __dir,
> struct __old_dirent64 *** __namelist,
> int (*__selector) (const struct __old_dirent64 *),
Ok.
> diff --git a/sysdeps/unix/sysv/linux/readdir64.c b/sysdeps/unix/sysv/linux/readdir64.c
> index e6f5108c0a..e6b8867b7a 100644
> --- a/sysdeps/unix/sysv/linux/readdir64.c
> +++ b/sysdeps/unix/sysv/linux/readdir64.c
> @@ -26,17 +26,13 @@
> #undef __readdir
> #undef readdir
>
> -/* Read a directory entry from DIRP. */
> -struct dirent64 *
> -__readdir64 (DIR *dirp)
> +/* Read a directory entry from DIRP. No locking. */
> +static struct dirent64 *
> +__readdir64_unlocked (DIR *dirp)
Ok.
> {
> struct dirent64 *dp;
> int saved_errno = errno;
>
> -#if IS_IN (libc)
> - __libc_lock_lock (dirp->lock);
> -#endif
> -
> if (dirp->offset >= dirp->size)
> {
> /* We've emptied out our buffer. Refill it. */
> @@ -53,9 +49,6 @@ __readdir64 (DIR *dirp)
> do not set errno in that case, to indicate success. */
> if (bytes == 0 || errno == ENOENT)
> __set_errno (saved_errno);
> -#if IS_IN (libc)
> - __libc_lock_unlock (dirp->lock);
> -#endif
> return NULL;
> }
> dirp->size = (size_t) bytes;
> @@ -68,10 +61,16 @@ __readdir64 (DIR *dirp)
> dirp->offset += dp->d_reclen;
> dirp->filepos = dp->d_off;
>
> -#if IS_IN (libc)
> - __libc_lock_unlock (dirp->lock);
> -#endif
> + return dp;
> +}
Ok.
> +/* Read a directory entry from DIRP. */
> +struct dirent64 *
> +__readdir64 (DIR *dirp)
> +{
> + __libc_lock_lock (dirp->lock);
> + struct dirent64 *dp = __readdir64_unlocked (dirp);
> + __libc_lock_unlock (dirp->lock);
> return dp;
> }
Ok.
> libc_hidden_def (__readdir64)
> @@ -99,45 +98,54 @@ __old_readdir64 (DIR *dirp)
> struct __old_dirent64 *dp;
> int saved_errno = errno;
>
> -#if IS_IN (libc)
> __libc_lock_lock (dirp->lock);
> -#endif
>
> - if (dirp->offset >= dirp->size)
> + while (1)
> {
> - /* We've emptied out our buffer. Refill it. */
> + errno = 0;
> + struct dirent64 *newdp = __readdir64_unlocked (dirp);
> + if (newdp == NULL)
> + {
> + if (errno == 0 && dirp->errcode != 0)
> + __set_errno (dirp->errcode);
> + else if (errno == 0)
> + __set_errno (saved_errno);
> + dp = NULL;
> + break;
> + }
If error, return error. Ok.
>
> - size_t maxread = dirp->allocation;
> - ssize_t bytes;
> + /* Convert to the target layout. Use a separate struct and
> + memcpy to side-step aliasing issues. */
> + struct __old_dirent64 result;
> + result.d_ino = newdp->d_ino;
> + result.d_off = newdp->d_off;
> + result.d_reclen = newdp->d_reclen;
> + result.d_type = newdp->d_type;
Copy new format to old format. Ok.
> - bytes = __old_getdents64 (dirp->fd, dirp->data, maxread);
> - if (bytes <= 0)
> + /* Check for ino_t overflow. */
> + if (__glibc_unlikely (result.d_ino != newdp->d_ino))
> + dirp->errcode = ENAMETOOLONG;
> + continue;
Check for truncation. Ok.
> {
> - /* Linux may fail with ENOENT on some file systems if the
> - directory inode is marked as dead (deleted). POSIX
> - treats this as a regular end-of-directory condition, so
> - do not set errno in that case, to indicate success. */
> - if (bytes == 0 || errno == ENOENT)
> - __set_errno (saved_errno);
> -#if IS_IN (libc)
> - __libc_lock_unlock (dirp->lock);
> -#endif
> - return NULL;
> }
> - dirp->size = (size_t) bytes;
>
> - /* Reset the offset into the buffer. */
> - dirp->offset = 0;
> - }
> - dp = (struct __old_dirent64 *) &dirp->data[dirp->offset];
> - dirp->offset += dp->d_reclen;
> - dirp->filepos = dp->d_off;
Ok.
> + /* Overwrite the fixed-sized part. */
> + dp = (struct __old_dirent64 *) newdp;
> + memcpy (dp, &result, offsetof (struct __old_dirent64, d_name));
Ok.
> + /* Move the name. */
> + _Static_assert (offsetof (struct __old_dirent64, d_name)
> + <= offsetof (struct dirent64, d_name),
> + "old struct must be smaller");
> + if (offsetof (struct __old_dirent64, d_name)
> + != offsetof (struct dirent64, d_name))
> + memmove (dp->d_name, newdp->d_name, strlen (newdp->d_name) + 1);
Ok.
> -#if IS_IN (libc)
> - __libc_lock_unlock (dirp->lock);
> -#endif
> + __set_errno (saved_errno);
> + break;
> + }
>
> + __libc_lock_unlock (dirp->lock);
> return dp;
> }
> libc_hidden_def (__old_readdir64)
Ok.
> diff --git a/sysdeps/unix/sysv/linux/readdir64_r.c b/sysdeps/unix/sysv/linux/readdir64_r.c
> __old_readdir64_r (DIR *dirp, struct __old_dirent64 *entry,
> struct __old_dirent64 **result)
> {
> - struct __old_dirent64 *dp;
> - size_t reclen;
> - const int saved_errno = errno;
> - int ret;
> -
> - __libc_lock_lock (dirp->lock);
> -
> - do
> - if (dirp->offset >= dirp->size)
> - {
> - /* We've emptied out our buffer. Refill it. */
> -
> - size_t maxread = dirp->allocation;
> - ssize_t bytes;
> -
> - maxread = dirp->allocation;
> -
> - bytes = __old_getdents64 (dirp->fd, dirp->data, maxread);
> - if (bytes <= 0)
> - {
> - /* On some systems getdents fails with ENOENT when the
> - open directory has been rmdir'd already. POSIX.1
> - requires that we treat this condition like normal EOF. */
> - if (bytes < 0 && errno == ENOENT)
> - {
> - bytes = 0;
> - __set_errno (saved_errno);
> - }
> - if (bytes < 0)
> - dirp->errcode = errno;
>
> - dp = NULL;
> - break;
> - }
> - dirp->size = (size_t) bytes;
> -
> - /* Reset the offset into the buffer. */
> - dirp->offset = 0;
> -
> - dp = (struct __old_dirent64 *) &dirp->data[dirp->offset];
> -
> - reclen = dp->d_reclen;
> -
> - dirp->offset += reclen;
> -
> - dirp->filepos = dp->d_off;
> -
> - if (reclen > offsetof (struct __old_dirent64, d_name) + NAME_MAX + 1)
> - /* The record is very long. It could still fit into the
> - caller-supplied buffer if we can skip padding at the
> - end. */
> - size_t namelen = _D_EXACT_NAMLEN (dp);
> - if (namelen <= NAME_MAX)
> - reclen = offsetof (struct __old_dirent64, d_name) + namelen + 1;
> - else
> - /* The name is too long. Ignore this file. */
> - dirp->errcode = ENAMETOOLONG;
> - dp->d_ino = 0;
> + while (1)
> {
> + struct dirent64 new_entry;
> + struct dirent64 *newp;
> + int ret = __readdir64_r (dirp, &new_entry, &newp);
> + if (ret != 0)
> + return ret;
error, ok.
> + else if (newp == NULL)
> + {
> + *result = NULL;
> + return 0;
> }
no more entries, ok.
> + else
> {
> + entry->d_ino = newp->d_ino;
> + if (entry->d_ino != newp->d_ino)
> {
> + dirp->errcode = EOVERFLOW;
> continue;
> }
Ok.
> + size_t namelen = strlen (newp->d_name);
> + entry->d_off = newp->d_off;
> + entry->d_reclen = (offsetof (struct __old_dirent64, d_name)
> + + namelen + 1);
> + entry->d_type = newp->d_type;
> + memcpy (entry->d_name, newp->d_name, namelen + 1);
> + *result = entry;
> + return 0;
> }
Ok.
> -
> - /* Skip deleted and ignored files. */
> - }
> - while (dp->d_ino == 0);
> -
> - if (dp != NULL)
> - {
> - *result = memcpy (entry, dp, reclen);
> - entry->d_reclen = reclen;
> - ret = 0;
> }
> - else
> - {
> - *result = NULL;
> - ret = dirp->errcode;
> - }
> -
> - __libc_lock_unlock (dirp->lock);
> -
> - return ret;
> }
>
> compat_symbol (libc, __old_readdir64_r, readdir64_r, GLIBC_2_1);
Ok.
More information about the Libc-alpha
mailing list