[PATCH] linux: mips: Fix getdents64 fallback on mips64-n32
Adhemerval Zanella
adhemerval.zanella@linaro.org
Fri Jan 22 14:20:50 GMT 2021
On 22/01/2021 09:49, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
>
>> Would it be more in line of what you have suggested?
>
> Yes, thanks.
>> while ((char *) kdp < (char *) skdp + r)
>> {
>> +
>> + /* Obtain the d_ino, d_off, and d_reclen from kernel filled buffer. */
>> + struct kernel_dirent kdirent;
>> + memcpy (&kdirent, kdp, sizeof (struct kernel_dirent));
>
> I believe this should use offsetof (struct kernel_dirent, d_name)
> for the size of the copy. Technically, the padding at the end of the
> struct may not be present in the buffer.
Right, I was working the assumption that kernel always returned at least
one byte do d_name ('\0').
>
>> @@ -118,19 +115,19 @@ __getdents64 (int fd, void *buf, size_t nbytes)
>> }
>> nb += new_reclen;
>>
>> + struct dirent64 d64;
>> + d64.d_ino = kdirent.d_ino;
>> + d64.d_off = kdirent.d_off;
>> + d64.d_reclen = new_reclen;
>> + d64.d_type = *((char *) kdp + kdirent.d_reclen - 1);
>> + memcpy (d64.d_name, kdp->d_name,
>> + kdirent.d_reclen - offsetof (struct kernel_dirent, d_name));
>> +
>> + memcpy (dp, &d64, new_reclen);
>> + last_offset = kdirent.d_off;
>>
>> dp = (struct dirent64 *) ((char *) dp + new_reclen);
>> + kdp = (struct kernel_dirent *) (((char *) kdp) + kdirent.d_reclen);
>> }
>
> I think the first memcpy is wrong: It has to go into the result buffer
> because only that will be large enough. 256 bytes for d_name may not be
> enough. The second memcpy should only copy the header (before d_name).
>
Indeed, I fixed on the patch below. The tst-getdents64 does pass on
mips64 and mips64-n32 on gccfarm machine (I explicit disabled the
getdents64 call for the test).
--
[PATCH] linux: mips: Fix getdents64 fallback on mips64-n32
GCC mainline shows the following error:
../sysdeps/unix/sysv/linux/mips/mips64/getdents64.c: In function '__getdents64':
../sysdeps/unix/sysv/linux/mips/mips64/getdents64.c:121:7: error: 'memcpy' forming offset [4, 7] is out of the bounds [0, 4] [-Werror=array-bounds]
121 | memcpy (((char *) dp + offsetof (struct dirent64, d_ino)),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
122 | KDP_MEMBER (kdp, d_ino), sizeof ((struct dirent64){0}.d_ino));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../sysdeps/unix/sysv/linux/mips/mips64/getdents64.c:123:7: error: 'memcpy' forming offset [4, 7] is out of the bounds [0, 4] [-Werror=array-bounds]
123 | memcpy (((char *) dp + offsetof (struct dirent64, d_off)),
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
124 | KDP_MEMBER (kdp, d_off), sizeof ((struct dirent64){0}.d_off));
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The issue is due both d_ino and d_off fields for mips64-n32
kernel_dirent are 32-bits, while this is using memcpy to copy 64 bits
from it into the glibc dirent64.
The fix is to use a temporary buffer to read the correct type
from kernel_dirent.
Checked with a build-many-glibcs.py for mips64el-linux-gnu and I
also checked the tst-getdents64 on mips64el 4.1.4 kernel with
and without fallback enabled (by manually setting the
getdents64_supported).
---
.../unix/sysv/linux/mips/mips64/getdents64.c | 37 +++++++++----------
sysdeps/unix/sysv/linux/tst-getdents64.c | 29 ++++++++++++---
2 files changed, 42 insertions(+), 24 deletions(-)
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c b/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
index a218f68104..ed6589ab94 100644
--- a/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
+++ b/sysdeps/unix/sysv/linux/mips/mips64/getdents64.c
@@ -90,17 +90,14 @@ __getdents64 (int fd, void *buf, size_t nbytes)
while ((char *) kdp < (char *) skdp + r)
{
- /* This macro is used to avoid aliasing violation. */
-#define KDP_MEMBER(src, member) \
- (__typeof__((struct kernel_dirent){0}.member) *) \
- memcpy (&((__typeof__((struct kernel_dirent){0}.member)){0}), \
- ((char *)(src) + offsetof (struct kernel_dirent, member)),\
- sizeof ((struct kernel_dirent){0}.member))
-
/* This is a conservative approximation, since some of size_diff might
fit into the existing padding for alignment. */
- unsigned short int k_reclen = *KDP_MEMBER (kdp, d_reclen);
- unsigned short int new_reclen = ALIGN_UP (k_reclen + size_diff,
+
+ /* Obtain the d_ino, d_off, and d_reclen from kernel filled buffer. */
+ struct kernel_dirent kdirent;
+ memcpy (&kdirent, kdp, offsetof (struct kernel_dirent, d_name));
+
+ unsigned short int new_reclen = ALIGN_UP (kdirent.d_reclen + size_diff,
_Alignof (struct dirent64));
if (nb + new_reclen > nbytes)
{
@@ -118,19 +115,21 @@ __getdents64 (int fd, void *buf, size_t nbytes)
}
nb += new_reclen;
- memcpy (((char *) dp + offsetof (struct dirent64, d_ino)),
- KDP_MEMBER (kdp, d_ino), sizeof ((struct dirent64){0}.d_ino));
- memcpy (((char *) dp + offsetof (struct dirent64, d_off)),
- KDP_MEMBER (kdp, d_off), sizeof ((struct dirent64){0}.d_off));
- last_offset = *KDP_MEMBER (kdp, d_off);
- memcpy (((char *) dp + offsetof (struct dirent64, d_reclen)),
- &new_reclen, sizeof (new_reclen));
- dp->d_type = *((char *) kdp + k_reclen - 1);
+ struct dirent64 d64;
+ d64.d_ino = kdirent.d_ino;
+ d64.d_off = kdirent.d_off;
+ d64.d_reclen = new_reclen;
+ d64.d_type = *((char *) kdp + kdirent.d_reclen - 1);
+ /* First copy only the header. */
+ memcpy (dp, &d64, offsetof (struct dirent64, d_name));
+ /* And then the d_name. */
memcpy (dp->d_name, kdp->d_name,
- k_reclen - offsetof (struct kernel_dirent, d_name));
+ kdirent.d_reclen - offsetof (struct kernel_dirent, d_name));
+
+ last_offset = kdirent.d_off;
dp = (struct dirent64 *) ((char *) dp + new_reclen);
- kdp = (struct kernel_dirent *) (((char *) kdp) + k_reclen);
+ kdp = (struct kernel_dirent *) (((char *) kdp) + kdirent.d_reclen);
}
return (char *) dp - (char *) buf;
diff --git a/sysdeps/unix/sysv/linux/tst-getdents64.c b/sysdeps/unix/sysv/linux/tst-getdents64.c
index 379ecbbcc6..691444d56e 100644
--- a/sysdeps/unix/sysv/linux/tst-getdents64.c
+++ b/sysdeps/unix/sysv/linux/tst-getdents64.c
@@ -76,8 +76,18 @@ large_buffer_checks (int fd)
}
}
-static int
-do_test (void)
+static void
+do_test_large_size (void)
+{
+ int fd = xopen (".", O_RDONLY | O_DIRECTORY, 0);
+ TEST_VERIFY (fd >= 0);
+ large_buffer_checks (fd);
+
+ xclose (fd);
+}
+
+static void
+do_test_by_size (size_t buffer_size)
{
/* The test compares the iteration order with readdir64. */
DIR *reference = opendir (".");
@@ -98,7 +108,7 @@ do_test (void)
non-existing data. */
struct
{
- char buffer[1024];
+ char buffer[buffer_size];
struct dirent64 pad;
} data;
@@ -153,10 +163,19 @@ do_test (void)
rewinddir (reference);
}
- large_buffer_checks (fd);
-
xclose (fd);
closedir (reference);
+}
+
+static int
+do_test (void)
+{
+ do_test_by_size (512);
+ do_test_by_size (1024);
+ do_test_by_size (4096);
+
+ do_test_large_size ();
+
return 0;
}
--
2.25.1
More information about the Libc-alpha
mailing list