* [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704)
@ 2021-11-14 10:35 Richard Henderson
2021-11-14 10:35 ` [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64 Richard Henderson
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Richard Henderson @ 2021-11-14 10:35 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent
There are a number of alignement issues flagged up by clang,
this attempts to fix only one of them: getdents.
Changes for v2:
* Do not QEMU_BUILD_BUG_ON for size mismatch,
as this triggers for i386 host.
r~
Richard Henderson (4):
linux-user: Split out do_getdents, do_getdents64
linux-user: Always use flexible arrays for dirent d_name
linux-user: Fix member types of target_dirent64
linux-user: Rewrite do_getdents, do_getdents64
linux-user/syscall_defs.h | 12 +-
linux-user/syscall.c | 314 +++++++++++++++++++-------------------
2 files changed, 165 insertions(+), 161 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64
2021-11-14 10:35 [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Richard Henderson
@ 2021-11-14 10:35 ` Richard Henderson
2021-11-14 21:09 ` Philippe Mathieu-Daudé
2021-11-14 10:35 ` [PATCH v2 2/4] linux-user: Always use flexible arrays for dirent d_name Richard Henderson
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2021-11-14 10:35 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent
Retain all 3 implementations of getdents for now.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 325 +++++++++++++++++++++++--------------------
1 file changed, 172 insertions(+), 153 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 544f5b662f..a2f605dec4 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8137,6 +8137,176 @@ static int host_to_target_cpu_mask(const unsigned long *host_mask,
return 0;
}
+#ifdef TARGET_NR_getdents
+static int do_getdents(abi_long arg1, abi_long arg2, abi_long arg3)
+{
+ int ret;
+
+#ifdef EMULATE_GETDENTS_WITH_GETDENTS
+# if TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
+ struct target_dirent *target_dirp;
+ struct linux_dirent *dirp;
+ abi_long count = arg3;
+
+ dirp = g_try_malloc(count);
+ if (!dirp) {
+ return -TARGET_ENOMEM;
+ }
+
+ ret = get_errno(sys_getdents(arg1, dirp, count));
+ if (!is_error(ret)) {
+ struct linux_dirent *de;
+ struct target_dirent *tde;
+ int len = ret;
+ int reclen, treclen;
+ int count1, tnamelen;
+
+ count1 = 0;
+ de = dirp;
+ target_dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
+ if (!target_dirp) {
+ return -TARGET_EFAULT;
+ }
+ tde = target_dirp;
+ while (len > 0) {
+ reclen = de->d_reclen;
+ tnamelen = reclen - offsetof(struct linux_dirent, d_name);
+ assert(tnamelen >= 0);
+ treclen = tnamelen + offsetof(struct target_dirent, d_name);
+ assert(count1 + treclen <= count);
+ tde->d_reclen = tswap16(treclen);
+ tde->d_ino = tswapal(de->d_ino);
+ tde->d_off = tswapal(de->d_off);
+ memcpy(tde->d_name, de->d_name, tnamelen);
+ de = (struct linux_dirent *)((char *)de + reclen);
+ len -= reclen;
+ tde = (struct target_dirent *)((char *)tde + treclen);
+ count1 += treclen;
+ }
+ ret = count1;
+ unlock_user(target_dirp, arg2, ret);
+ }
+ g_free(dirp);
+# else
+ struct linux_dirent *dirp;
+ abi_long count = arg3;
+
+ dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
+ if (!dirp) {
+ return -TARGET_EFAULT;
+ }
+ ret = get_errno(sys_getdents(arg1, dirp, count));
+ if (!is_error(ret)) {
+ struct linux_dirent *de;
+ int len = ret;
+ int reclen;
+ de = dirp;
+ while (len > 0) {
+ reclen = de->d_reclen;
+ if (reclen > len) {
+ break;
+ }
+ de->d_reclen = tswap16(reclen);
+ tswapls(&de->d_ino);
+ tswapls(&de->d_off);
+ de = (struct linux_dirent *)((char *)de + reclen);
+ len -= reclen;
+ }
+ }
+ unlock_user(dirp, arg2, ret);
+# endif
+#else
+ /* Implement getdents in terms of getdents64 */
+ struct linux_dirent64 *dirp;
+ abi_long count = arg3;
+
+ dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
+ if (!dirp) {
+ return -TARGET_EFAULT;
+ }
+ ret = get_errno(sys_getdents64(arg1, dirp, count));
+ if (!is_error(ret)) {
+ /*
+ * Convert the dirent64 structs to target dirent. We do this
+ * in-place, since we can guarantee that a target_dirent is no
+ * larger than a dirent64; however this means we have to be
+ * careful to read everything before writing in the new format.
+ */
+ struct linux_dirent64 *de;
+ struct target_dirent *tde;
+ int len = ret;
+ int tlen = 0;
+
+ de = dirp;
+ tde = (struct target_dirent *)dirp;
+ while (len > 0) {
+ int namelen, treclen;
+ int reclen = de->d_reclen;
+ uint64_t ino = de->d_ino;
+ int64_t off = de->d_off;
+ uint8_t type = de->d_type;
+
+ namelen = strlen(de->d_name);
+ treclen = offsetof(struct target_dirent, d_name) + namelen + 2;
+ treclen = QEMU_ALIGN_UP(treclen, sizeof(abi_long));
+
+ memmove(tde->d_name, de->d_name, namelen + 1);
+ tde->d_ino = tswapal(ino);
+ tde->d_off = tswapal(off);
+ tde->d_reclen = tswap16(treclen);
+ /*
+ * The target_dirent type is in what was formerly a padding
+ * byte at the end of the structure:
+ */
+ *(((char *)tde) + treclen - 1) = type;
+
+ de = (struct linux_dirent64 *)((char *)de + reclen);
+ tde = (struct target_dirent *)((char *)tde + treclen);
+ len -= reclen;
+ tlen += treclen;
+ }
+ ret = tlen;
+ }
+ unlock_user(dirp, arg2, ret);
+#endif
+ return ret;
+}
+#endif /* TARGET_NR_getdents */
+
+#if defined(TARGET_NR_getdents64) && defined(__NR_getdents64)
+static int do_getdents64(abi_long arg1, abi_long arg2, abi_long arg3)
+{
+ struct linux_dirent64 *dirp;
+ abi_long count = arg3;
+ int ret;
+
+ dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
+ if (!dirp) {
+ return -TARGET_EFAULT;
+ }
+ ret = get_errno(sys_getdents64(arg1, dirp, count));
+ if (!is_error(ret)) {
+ struct linux_dirent64 *de;
+ int len = ret;
+ int reclen;
+ de = dirp;
+ while (len > 0) {
+ reclen = de->d_reclen;
+ if (reclen > len) {
+ break;
+ }
+ de->d_reclen = tswap16(reclen);
+ tswap64s((uint64_t *)&de->d_ino);
+ tswap64s((uint64_t *)&de->d_off);
+ de = (struct linux_dirent64 *)((char *)de + reclen);
+ len -= reclen;
+ }
+ }
+ unlock_user(dirp, arg2, ret);
+ return ret;
+}
+#endif /* TARGET_NR_getdents64 */
+
#if defined(TARGET_NR_pivot_root) && defined(__NR_pivot_root)
_syscall2(int, pivot_root, const char *, new_root, const char *, put_old)
#endif
@@ -10227,162 +10397,11 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
#endif
#ifdef TARGET_NR_getdents
case TARGET_NR_getdents:
-#ifdef EMULATE_GETDENTS_WITH_GETDENTS
-#if TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
- {
- struct target_dirent *target_dirp;
- struct linux_dirent *dirp;
- abi_long count = arg3;
-
- dirp = g_try_malloc(count);
- if (!dirp) {
- return -TARGET_ENOMEM;
- }
-
- ret = get_errno(sys_getdents(arg1, dirp, count));
- if (!is_error(ret)) {
- struct linux_dirent *de;
- struct target_dirent *tde;
- int len = ret;
- int reclen, treclen;
- int count1, tnamelen;
-
- count1 = 0;
- de = dirp;
- if (!(target_dirp = lock_user(VERIFY_WRITE, arg2, count, 0)))
- return -TARGET_EFAULT;
- tde = target_dirp;
- while (len > 0) {
- reclen = de->d_reclen;
- tnamelen = reclen - offsetof(struct linux_dirent, d_name);
- assert(tnamelen >= 0);
- treclen = tnamelen + offsetof(struct target_dirent, d_name);
- assert(count1 + treclen <= count);
- tde->d_reclen = tswap16(treclen);
- tde->d_ino = tswapal(de->d_ino);
- tde->d_off = tswapal(de->d_off);
- memcpy(tde->d_name, de->d_name, tnamelen);
- de = (struct linux_dirent *)((char *)de + reclen);
- len -= reclen;
- tde = (struct target_dirent *)((char *)tde + treclen);
- count1 += treclen;
- }
- ret = count1;
- unlock_user(target_dirp, arg2, ret);
- }
- g_free(dirp);
- }
-#else
- {
- struct linux_dirent *dirp;
- abi_long count = arg3;
-
- if (!(dirp = lock_user(VERIFY_WRITE, arg2, count, 0)))
- return -TARGET_EFAULT;
- ret = get_errno(sys_getdents(arg1, dirp, count));
- if (!is_error(ret)) {
- struct linux_dirent *de;
- int len = ret;
- int reclen;
- de = dirp;
- while (len > 0) {
- reclen = de->d_reclen;
- if (reclen > len)
- break;
- de->d_reclen = tswap16(reclen);
- tswapls(&de->d_ino);
- tswapls(&de->d_off);
- de = (struct linux_dirent *)((char *)de + reclen);
- len -= reclen;
- }
- }
- unlock_user(dirp, arg2, ret);
- }
-#endif
-#else
- /* Implement getdents in terms of getdents64 */
- {
- struct linux_dirent64 *dirp;
- abi_long count = arg3;
-
- dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
- if (!dirp) {
- return -TARGET_EFAULT;
- }
- ret = get_errno(sys_getdents64(arg1, dirp, count));
- if (!is_error(ret)) {
- /* Convert the dirent64 structs to target dirent. We do this
- * in-place, since we can guarantee that a target_dirent is no
- * larger than a dirent64; however this means we have to be
- * careful to read everything before writing in the new format.
- */
- struct linux_dirent64 *de;
- struct target_dirent *tde;
- int len = ret;
- int tlen = 0;
-
- de = dirp;
- tde = (struct target_dirent *)dirp;
- while (len > 0) {
- int namelen, treclen;
- int reclen = de->d_reclen;
- uint64_t ino = de->d_ino;
- int64_t off = de->d_off;
- uint8_t type = de->d_type;
-
- namelen = strlen(de->d_name);
- treclen = offsetof(struct target_dirent, d_name)
- + namelen + 2;
- treclen = QEMU_ALIGN_UP(treclen, sizeof(abi_long));
-
- memmove(tde->d_name, de->d_name, namelen + 1);
- tde->d_ino = tswapal(ino);
- tde->d_off = tswapal(off);
- tde->d_reclen = tswap16(treclen);
- /* The target_dirent type is in what was formerly a padding
- * byte at the end of the structure:
- */
- *(((char *)tde) + treclen - 1) = type;
-
- de = (struct linux_dirent64 *)((char *)de + reclen);
- tde = (struct target_dirent *)((char *)tde + treclen);
- len -= reclen;
- tlen += treclen;
- }
- ret = tlen;
- }
- unlock_user(dirp, arg2, ret);
- }
-#endif
- return ret;
+ return do_getdents(arg1, arg2, arg3);
#endif /* TARGET_NR_getdents */
#if defined(TARGET_NR_getdents64) && defined(__NR_getdents64)
case TARGET_NR_getdents64:
- {
- struct linux_dirent64 *dirp;
- abi_long count = arg3;
- if (!(dirp = lock_user(VERIFY_WRITE, arg2, count, 0)))
- return -TARGET_EFAULT;
- ret = get_errno(sys_getdents64(arg1, dirp, count));
- if (!is_error(ret)) {
- struct linux_dirent64 *de;
- int len = ret;
- int reclen;
- de = dirp;
- while (len > 0) {
- reclen = de->d_reclen;
- if (reclen > len)
- break;
- de->d_reclen = tswap16(reclen);
- tswap64s((uint64_t *)&de->d_ino);
- tswap64s((uint64_t *)&de->d_off);
- de = (struct linux_dirent64 *)((char *)de + reclen);
- len -= reclen;
- }
- }
- unlock_user(dirp, arg2, ret);
- }
- return ret;
+ return do_getdents64(arg1, arg2, arg3);
#endif /* TARGET_NR_getdents64 */
#if defined(TARGET_NR__newselect)
case TARGET_NR__newselect:
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 2/4] linux-user: Always use flexible arrays for dirent d_name
2021-11-14 10:35 [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Richard Henderson
2021-11-14 10:35 ` [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64 Richard Henderson
@ 2021-11-14 10:35 ` Richard Henderson
2021-11-14 21:10 ` Philippe Mathieu-Daudé
2021-11-14 10:35 ` [PATCH v2 3/4] linux-user: Fix member types of target_dirent64 Richard Henderson
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2021-11-14 10:35 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent
We currently use a flexible array member for target_dirent,
but use incorrectly fixed length arrays for target_dirent64,
linux_dirent and linux_dirent64.
This requires that we adjust the definition of the VFAT READDIR
ioctls which hard-code the 256 namelen size into the ioctl constant.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall_defs.h | 6 +++---
linux-user/syscall.c | 6 ++++--
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index a5ce487dcc..98b09ee6d6 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -441,7 +441,7 @@ struct target_dirent64 {
int64_t d_off;
unsigned short d_reclen;
unsigned char d_type;
- char d_name[256];
+ char d_name[];
};
@@ -2714,7 +2714,7 @@ struct linux_dirent {
long d_ino;
unsigned long d_off;
unsigned short d_reclen;
- char d_name[256]; /* We must not include limits.h! */
+ char d_name[];
};
struct linux_dirent64 {
@@ -2722,7 +2722,7 @@ struct linux_dirent64 {
int64_t d_off;
unsigned short d_reclen;
unsigned char d_type;
- char d_name[256];
+ char d_name[];
};
struct target_mq_attr {
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a2f605dec4..499415ad81 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -197,8 +197,10 @@
//#define DEBUG_ERESTARTSYS
//#include <linux/msdos_fs.h>
-#define VFAT_IOCTL_READDIR_BOTH _IOR('r', 1, struct linux_dirent [2])
-#define VFAT_IOCTL_READDIR_SHORT _IOR('r', 2, struct linux_dirent [2])
+#define VFAT_IOCTL_READDIR_BOTH \
+ _IOC(_IOC_READ, 'r', 1, (sizeof(struct linux_dirent) + 256) * 2)
+#define VFAT_IOCTL_READDIR_SHORT \
+ _IOC(_IOC_READ, 'r', 2, (sizeof(struct linux_dirent) + 256) * 2)
#undef _syscall0
#undef _syscall1
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/4] linux-user: Fix member types of target_dirent64
2021-11-14 10:35 [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Richard Henderson
2021-11-14 10:35 ` [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64 Richard Henderson
2021-11-14 10:35 ` [PATCH v2 2/4] linux-user: Always use flexible arrays for dirent d_name Richard Henderson
@ 2021-11-14 10:35 ` Richard Henderson
2021-11-14 20:57 ` Philippe Mathieu-Daudé
2021-11-14 10:35 ` [PATCH v2 4/4] linux-user: Rewrite do_getdents, do_getdents64 Richard Henderson
2021-11-22 8:16 ` [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Laurent Vivier
4 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2021-11-14 10:35 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent
The host uint64_t (etc) does not have the correct
alignment constraint as the guest: use abi_* types.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall_defs.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 98b09ee6d6..41aaafbac1 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -437,9 +437,9 @@ struct target_dirent {
};
struct target_dirent64 {
- uint64_t d_ino;
- int64_t d_off;
- unsigned short d_reclen;
+ abi_ullong d_ino;
+ abi_llong d_off;
+ abi_ushort d_reclen;
unsigned char d_type;
char d_name[];
};
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/4] linux-user: Rewrite do_getdents, do_getdents64
2021-11-14 10:35 [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Richard Henderson
` (2 preceding siblings ...)
2021-11-14 10:35 ` [PATCH v2 3/4] linux-user: Fix member types of target_dirent64 Richard Henderson
@ 2021-11-14 10:35 ` Richard Henderson
2021-11-14 21:18 ` Philippe Mathieu-Daudé
2021-11-22 8:16 ` [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Laurent Vivier
4 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2021-11-14 10:35 UTC (permalink / raw)
To: qemu-devel; +Cc: laurent
Always allocate host storage; this ensures that the struct
is sufficiently aligned for the host. Merge the three host
implementations of getdents via a few ifdefs. Utilize the
same method for do_getdents64.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/704
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
linux-user/syscall.c | 259 ++++++++++++++++++++-----------------------
1 file changed, 121 insertions(+), 138 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 499415ad81..f1cfcc8104 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8140,172 +8140,155 @@ static int host_to_target_cpu_mask(const unsigned long *host_mask,
}
#ifdef TARGET_NR_getdents
-static int do_getdents(abi_long arg1, abi_long arg2, abi_long arg3)
+static int do_getdents(abi_long dirfd, abi_long arg2, abi_long count)
{
- int ret;
+ g_autofree void *hdirp = NULL;
+ void *tdirp;
+ int hlen, hoff, toff;
+ int hreclen, treclen;
+ off64_t prev_diroff = 0;
-#ifdef EMULATE_GETDENTS_WITH_GETDENTS
-# if TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64
- struct target_dirent *target_dirp;
- struct linux_dirent *dirp;
- abi_long count = arg3;
-
- dirp = g_try_malloc(count);
- if (!dirp) {
+ hdirp = g_try_malloc(count);
+ if (!hdirp) {
return -TARGET_ENOMEM;
}
- ret = get_errno(sys_getdents(arg1, dirp, count));
- if (!is_error(ret)) {
- struct linux_dirent *de;
- struct target_dirent *tde;
- int len = ret;
- int reclen, treclen;
- int count1, tnamelen;
+#ifdef EMULATE_GETDENTS_WITH_GETDENTS
+ hlen = sys_getdents(dirfd, hdirp, count);
+#else
+ hlen = sys_getdents64(dirfd, hdirp, count);
+#endif
- count1 = 0;
- de = dirp;
- target_dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
- if (!target_dirp) {
- return -TARGET_EFAULT;
- }
- tde = target_dirp;
- while (len > 0) {
- reclen = de->d_reclen;
- tnamelen = reclen - offsetof(struct linux_dirent, d_name);
- assert(tnamelen >= 0);
- treclen = tnamelen + offsetof(struct target_dirent, d_name);
- assert(count1 + treclen <= count);
- tde->d_reclen = tswap16(treclen);
- tde->d_ino = tswapal(de->d_ino);
- tde->d_off = tswapal(de->d_off);
- memcpy(tde->d_name, de->d_name, tnamelen);
- de = (struct linux_dirent *)((char *)de + reclen);
- len -= reclen;
- tde = (struct target_dirent *)((char *)tde + treclen);
- count1 += treclen;
- }
- ret = count1;
- unlock_user(target_dirp, arg2, ret);
+ hlen = get_errno(hlen);
+ if (is_error(hlen)) {
+ return hlen;
}
- g_free(dirp);
-# else
- struct linux_dirent *dirp;
- abi_long count = arg3;
- dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
- if (!dirp) {
+ tdirp = lock_user(VERIFY_WRITE, arg2, count, 0);
+ if (!tdirp) {
return -TARGET_EFAULT;
}
- ret = get_errno(sys_getdents(arg1, dirp, count));
- if (!is_error(ret)) {
- struct linux_dirent *de;
- int len = ret;
- int reclen;
- de = dirp;
- while (len > 0) {
- reclen = de->d_reclen;
- if (reclen > len) {
+
+ for (hoff = toff = 0; hoff < hlen; hoff += hreclen, toff += treclen) {
+#ifdef EMULATE_GETDENTS_WITH_GETDENTS
+ struct linux_dirent *hde = hdirp + hoff;
+#else
+ struct linux_dirent64 *hde = hdirp + hoff;
+#endif
+ struct target_dirent *tde = tdirp + toff;
+ int namelen;
+ uint8_t type;
+
+ namelen = strlen(hde->d_name);
+ hreclen = hde->d_reclen;
+ treclen = offsetof(struct target_dirent, d_name) + namelen + 2;
+ treclen = QEMU_ALIGN_UP(treclen, __alignof(struct target_dirent));
+
+ if (toff + treclen > count) {
+ /*
+ * If the host struct is smaller than the target struct, or
+ * requires less alignment and thus packs into less space,
+ * then the host can return more entries than we can pass
+ * on to the guest.
+ */
+ if (toff == 0) {
+ toff = -TARGET_EINVAL; /* result buffer is too small */
break;
}
- de->d_reclen = tswap16(reclen);
- tswapls(&de->d_ino);
- tswapls(&de->d_off);
- de = (struct linux_dirent *)((char *)de + reclen);
- len -= reclen;
- }
- }
- unlock_user(dirp, arg2, ret);
-# endif
-#else
- /* Implement getdents in terms of getdents64 */
- struct linux_dirent64 *dirp;
- abi_long count = arg3;
-
- dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
- if (!dirp) {
- return -TARGET_EFAULT;
- }
- ret = get_errno(sys_getdents64(arg1, dirp, count));
- if (!is_error(ret)) {
- /*
- * Convert the dirent64 structs to target dirent. We do this
- * in-place, since we can guarantee that a target_dirent is no
- * larger than a dirent64; however this means we have to be
- * careful to read everything before writing in the new format.
- */
- struct linux_dirent64 *de;
- struct target_dirent *tde;
- int len = ret;
- int tlen = 0;
-
- de = dirp;
- tde = (struct target_dirent *)dirp;
- while (len > 0) {
- int namelen, treclen;
- int reclen = de->d_reclen;
- uint64_t ino = de->d_ino;
- int64_t off = de->d_off;
- uint8_t type = de->d_type;
-
- namelen = strlen(de->d_name);
- treclen = offsetof(struct target_dirent, d_name) + namelen + 2;
- treclen = QEMU_ALIGN_UP(treclen, sizeof(abi_long));
-
- memmove(tde->d_name, de->d_name, namelen + 1);
- tde->d_ino = tswapal(ino);
- tde->d_off = tswapal(off);
- tde->d_reclen = tswap16(treclen);
/*
- * The target_dirent type is in what was formerly a padding
- * byte at the end of the structure:
+ * Return what we have, resetting the file pointer to the
+ * location of the first record not returned.
*/
- *(((char *)tde) + treclen - 1) = type;
-
- de = (struct linux_dirent64 *)((char *)de + reclen);
- tde = (struct target_dirent *)((char *)tde + treclen);
- len -= reclen;
- tlen += treclen;
+ lseek64(dirfd, prev_diroff, SEEK_SET);
+ break;
}
- ret = tlen;
- }
- unlock_user(dirp, arg2, ret);
+
+ prev_diroff = hde->d_off;
+ tde->d_ino = tswapal(hde->d_ino);
+ tde->d_off = tswapal(hde->d_off);
+ tde->d_reclen = tswap16(treclen);
+ memcpy(tde->d_name, hde->d_name, namelen + 1);
+
+ /*
+ * The getdents type is in what was formerly a padding byte at the
+ * end of the structure.
+ */
+#ifdef EMULATE_GETDENTS_WITH_GETDENTS
+ type = *((uint8_t *)hde + hreclen - 1);
+#else
+ type = hde->d_type;
#endif
- return ret;
+ *((uint8_t *)tde + treclen - 1) = type;
+ }
+
+ unlock_user(tdirp, arg2, toff);
+ return toff;
}
#endif /* TARGET_NR_getdents */
#if defined(TARGET_NR_getdents64) && defined(__NR_getdents64)
-static int do_getdents64(abi_long arg1, abi_long arg2, abi_long arg3)
+static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count)
{
- struct linux_dirent64 *dirp;
- abi_long count = arg3;
- int ret;
+ g_autofree void *hdirp = NULL;
+ void *tdirp;
+ int hlen, hoff, toff;
+ int hreclen, treclen;
+ off64_t prev_diroff = 0;
- dirp = lock_user(VERIFY_WRITE, arg2, count, 0);
- if (!dirp) {
+ hdirp = g_try_malloc(count);
+ if (!hdirp) {
+ return -TARGET_ENOMEM;
+ }
+
+ hlen = get_errno(sys_getdents64(dirfd, hdirp, count));
+ if (is_error(hlen)) {
+ return hlen;
+ }
+
+ tdirp = lock_user(VERIFY_WRITE, arg2, count, 0);
+ if (!tdirp) {
return -TARGET_EFAULT;
}
- ret = get_errno(sys_getdents64(arg1, dirp, count));
- if (!is_error(ret)) {
- struct linux_dirent64 *de;
- int len = ret;
- int reclen;
- de = dirp;
- while (len > 0) {
- reclen = de->d_reclen;
- if (reclen > len) {
+
+ for (hoff = toff = 0; hoff < hlen; hoff += hreclen, toff += treclen) {
+ struct linux_dirent64 *hde = hdirp + hoff;
+ struct target_dirent64 *tde = tdirp + toff;
+ int namelen;
+
+ namelen = strlen(hde->d_name) + 1;
+ hreclen = hde->d_reclen;
+ treclen = offsetof(struct target_dirent64, d_name) + namelen;
+ treclen = QEMU_ALIGN_UP(treclen, __alignof(struct target_dirent64));
+
+ if (toff + treclen > count) {
+ /*
+ * If the host struct is smaller than the target struct, or
+ * requires less alignment and thus packs into less space,
+ * then the host can return more entries than we can pass
+ * on to the guest.
+ */
+ if (toff == 0) {
+ toff = -TARGET_EINVAL; /* result buffer is too small */
break;
}
- de->d_reclen = tswap16(reclen);
- tswap64s((uint64_t *)&de->d_ino);
- tswap64s((uint64_t *)&de->d_off);
- de = (struct linux_dirent64 *)((char *)de + reclen);
- len -= reclen;
+ /*
+ * Return what we have, resetting the file pointer to the
+ * location of the first record not returned.
+ */
+ lseek64(dirfd, prev_diroff, SEEK_SET);
+ break;
}
+
+ prev_diroff = hde->d_off;
+ tde->d_ino = tswap64(hde->d_ino);
+ tde->d_off = tswap64(hde->d_off);
+ tde->d_reclen = tswap16(treclen);
+ tde->d_type = hde->d_type;
+ memcpy(tde->d_name, hde->d_name, namelen);
}
- unlock_user(dirp, arg2, ret);
- return ret;
+
+ unlock_user(tdirp, arg2, toff);
+ return toff;
}
#endif /* TARGET_NR_getdents64 */
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 3/4] linux-user: Fix member types of target_dirent64
2021-11-14 10:35 ` [PATCH v2 3/4] linux-user: Fix member types of target_dirent64 Richard Henderson
@ 2021-11-14 20:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-14 20:57 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: laurent
On 11/14/21 11:35, Richard Henderson wrote:
> The host uint64_t (etc) does not have the correct
> alignment constraint as the guest: use abi_* types.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall_defs.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64
2021-11-14 10:35 ` [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64 Richard Henderson
@ 2021-11-14 21:09 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-14 21:09 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: laurent
On 11/14/21 11:35, Richard Henderson wrote:
> Retain all 3 implementations of getdents for now.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 325 +++++++++++++++++++++++--------------------
> 1 file changed, 172 insertions(+), 153 deletions(-)
Same as v1:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/4] linux-user: Always use flexible arrays for dirent d_name
2021-11-14 10:35 ` [PATCH v2 2/4] linux-user: Always use flexible arrays for dirent d_name Richard Henderson
@ 2021-11-14 21:10 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-14 21:10 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: laurent
On 11/14/21 11:35, Richard Henderson wrote:
> We currently use a flexible array member for target_dirent,
> but use incorrectly fixed length arrays for target_dirent64,
> linux_dirent and linux_dirent64.
>
> This requires that we adjust the definition of the VFAT READDIR
> ioctls which hard-code the 256 namelen size into the ioctl constant.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall_defs.h | 6 +++---
> linux-user/syscall.c | 6 ++++--
> 2 files changed, 7 insertions(+), 5 deletions(-)
Same as v1:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 4/4] linux-user: Rewrite do_getdents, do_getdents64
2021-11-14 10:35 ` [PATCH v2 4/4] linux-user: Rewrite do_getdents, do_getdents64 Richard Henderson
@ 2021-11-14 21:18 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-11-14 21:18 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: laurent
On 11/14/21 11:35, Richard Henderson wrote:
> Always allocate host storage; this ensures that the struct
> is sufficiently aligned for the host. Merge the three host
> implementations of getdents via a few ifdefs. Utilize the
> same method for do_getdents64.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/704
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> linux-user/syscall.c | 259 ++++++++++++++++++++-----------------------
> 1 file changed, 121 insertions(+), 138 deletions(-)
> + namelen = strlen(hde->d_name);
> + hreclen = hde->d_reclen;
> + treclen = offsetof(struct target_dirent, d_name) + namelen + 2;
> + treclen = QEMU_ALIGN_UP(treclen, __alignof(struct target_dirent));
> +
> + if (toff + treclen > count) {
> + /*
> + * If the host struct is smaller than the target struct, or
> + * requires less alignment and thus packs into less space,
> + * then the host can return more entries than we can pass
> + * on to the guest.
> + */
> + if (toff == 0) {
> + toff = -TARGET_EINVAL; /* result buffer is too small */
> break;
> }
[...]
> /*
> - * The target_dirent type is in what was formerly a padding
> - * byte at the end of the structure:
> + * Return what we have, resetting the file pointer to the
> + * location of the first record not returned.
> */
> + lseek64(dirfd, prev_diroff, SEEK_SET);
> + break;
> }
LGTM,
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704)
2021-11-14 10:35 [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Richard Henderson
` (3 preceding siblings ...)
2021-11-14 10:35 ` [PATCH v2 4/4] linux-user: Rewrite do_getdents, do_getdents64 Richard Henderson
@ 2021-11-22 8:16 ` Laurent Vivier
4 siblings, 0 replies; 10+ messages in thread
From: Laurent Vivier @ 2021-11-22 8:16 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Le 14/11/2021 à 11:35, Richard Henderson a écrit :
> There are a number of alignement issues flagged up by clang,
> this attempts to fix only one of them: getdents.
>
> Changes for v2:
> * Do not QEMU_BUILD_BUG_ON for size mismatch,
> as this triggers for i386 host.
>
>
> r~
>
> Richard Henderson (4):
> linux-user: Split out do_getdents, do_getdents64
> linux-user: Always use flexible arrays for dirent d_name
> linux-user: Fix member types of target_dirent64
> linux-user: Rewrite do_getdents, do_getdents64
>
> linux-user/syscall_defs.h | 12 +-
> linux-user/syscall.c | 314 +++++++++++++++++++-------------------
> 2 files changed, 165 insertions(+), 161 deletions(-)
>
Applied to my linux-user-for-6.2 branch.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-11-22 8:17 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-14 10:35 [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Richard Henderson
2021-11-14 10:35 ` [PATCH v2 1/4] linux-user: Split out do_getdents, do_getdents64 Richard Henderson
2021-11-14 21:09 ` Philippe Mathieu-Daudé
2021-11-14 10:35 ` [PATCH v2 2/4] linux-user: Always use flexible arrays for dirent d_name Richard Henderson
2021-11-14 21:10 ` Philippe Mathieu-Daudé
2021-11-14 10:35 ` [PATCH v2 3/4] linux-user: Fix member types of target_dirent64 Richard Henderson
2021-11-14 20:57 ` Philippe Mathieu-Daudé
2021-11-14 10:35 ` [PATCH v2 4/4] linux-user: Rewrite do_getdents, do_getdents64 Richard Henderson
2021-11-14 21:18 ` Philippe Mathieu-Daudé
2021-11-22 8:16 ` [PATCH v2 0/4] linux-user: Fix getdents alignment issues (#704) Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).