configure | 10 ++++ linux-user/ioctls.h | 5 ++ linux-user/syscall.c | 117 +++++++++++++++++++++++++++++++++++++ linux-user/syscall_defs.h | 15 +++++ linux-user/syscall_types.h | 11 ++++ 5 files changed, 158 insertions(+)
From: Chen Gang <chengang@emindsoft.com.cn>
Another DRM_IOCTL_* commands will be done later.
Signed-off-by: Chen Gang <chengang@emindsoft.com.cn>
---
configure | 10 ++++
linux-user/ioctls.h | 5 ++
linux-user/syscall.c | 117 +++++++++++++++++++++++++++++++++++++
linux-user/syscall_defs.h | 15 +++++
linux-user/syscall_types.h | 11 ++++
5 files changed, 158 insertions(+)
diff --git a/configure b/configure
index e225a1e3ff..3cf28a649a 100755
--- a/configure
+++ b/configure
@@ -3140,6 +3140,13 @@ if ! check_include "ifaddrs.h" ; then
have_ifaddrs_h=no
fi
+#########################################
+# libdrm check
+have_drm_h=no
+if check_include "libdrm/drm.h" ; then
+ have_drm_h=yes
+fi
+
##########################################
# VTE probe
@@ -7149,6 +7156,9 @@ fi
if test "$have_ifaddrs_h" = "yes" ; then
echo "HAVE_IFADDRS_H=y" >> $config_host_mak
fi
+if test "$have_drm_h" = "yes" ; then
+ echo "HAVE_DRM_H=y" >> $config_host_mak
+fi
if test "$have_broken_size_max" = "yes" ; then
echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
fi
diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 0defa1d8c1..f2e2fa9c87 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -574,6 +574,11 @@
IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt,
MK_PTR(MK_STRUCT(STRUCT_rtentry)))
+#ifdef HAVE_DRM_H
+ IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm,
+ MK_PTR(MK_STRUCT(STRUCT_drm_version)))
+#endif
+
#ifdef TARGET_TIOCSTART
IOCTL_IGNORE(TIOCSTART)
IOCTL_IGNORE(TIOCSTOP)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5af55fca78..07b7596e0f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -112,6 +112,9 @@
#include <linux/if_alg.h>
#include <linux/rtc.h>
#include <sound/asound.h>
+#ifdef HAVE_DRM_H
+#include <libdrm/drm.h>
+#endif
#include "linux_loop.h"
#include "uname.h"
@@ -5275,6 +5278,120 @@ static abi_long do_ioctl_tiocgptpeer(const IOCTLEntry *ie, uint8_t *buf_temp,
}
#endif
+#ifdef HAVE_DRM_H
+
+static void unlock_drm_version(struct drm_version *host_ver)
+{
+ if (host_ver->name) {
+ unlock_user(host_ver->name, 0UL, 0);
+ }
+ if (host_ver->date) {
+ unlock_user(host_ver->date, 0UL, 0);
+ }
+ if (host_ver->desc) {
+ unlock_user(host_ver->desc, 0UL, 0);
+ }
+}
+
+static inline abi_long target_to_host_drmversion(struct drm_version *host_ver,
+ abi_long target_addr)
+{
+ struct target_drm_version *target_ver;
+
+ if (!lock_user_struct(VERIFY_READ, target_ver, target_addr, 0)) {
+ return -TARGET_EFAULT;
+ }
+
+ memset(host_ver, 0, sizeof(*host_ver));
+
+ __get_user(host_ver->name_len, &target_ver->name_len);
+ if (host_ver->name_len) {
+ host_ver->name = lock_user(VERIFY_WRITE, target_ver->name,
+ target_ver->name_len, 0);
+ if (!host_ver->name) {
+ goto err;
+ }
+ }
+
+ __get_user(host_ver->date_len, &target_ver->date_len);
+ if (host_ver->date_len) {
+ host_ver->date = lock_user(VERIFY_WRITE, target_ver->date,
+ target_ver->date_len, 0);
+ if (!host_ver->date) {
+ goto err;
+ }
+ }
+
+ __get_user(host_ver->desc_len, &target_ver->desc_len);
+ if (host_ver->desc_len) {
+ host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
+ target_ver->desc_len, 0);
+ if (!host_ver->desc) {
+ goto err;
+ }
+ }
+
+ unlock_user_struct(target_ver, target_addr, 0);
+ return 0;
+err:
+ unlock_drm_version(host_ver);
+ unlock_user_struct(target_ver, target_addr, 0);
+ return -ENOMEM;
+}
+
+static inline abi_long host_to_target_drmversion(abi_ulong target_addr,
+ struct drm_version *host_ver)
+{
+ struct target_drm_version *target_ver;
+
+ if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) {
+ unlock_drm_version(host_ver);
+ return -TARGET_EFAULT;
+ }
+ __put_user(host_ver->version_major, &target_ver->version_major);
+ __put_user(host_ver->version_minor, &target_ver->version_minor);
+ __put_user(host_ver->version_patchlevel, &target_ver->version_patchlevel);
+ __put_user(host_ver->name_len, &target_ver->name_len);
+ __put_user(host_ver->date_len, &target_ver->date_len);
+ __put_user(host_ver->desc_len, &target_ver->desc_len);
+ if (host_ver->name) {
+ unlock_user(host_ver->name, target_ver->name, host_ver->name_len);
+ }
+ if (host_ver->date) {
+ unlock_user(host_ver->date, target_ver->date, host_ver->date_len);
+ }
+ if (host_ver->desc) {
+ unlock_user(host_ver->desc, target_ver->desc, host_ver->desc_len);
+ }
+ unlock_user_struct(target_ver, target_addr, 0);
+ return 0;
+}
+
+static abi_long do_ioctl_drm(const IOCTLEntry *ie, uint8_t *buf_temp,
+ int fd, int cmd, abi_long arg)
+{
+ struct drm_version *ver;
+ abi_long ret;
+
+ switch (ie->host_cmd) {
+ case DRM_IOCTL_VERSION:
+ ver = (struct drm_version *)buf_temp;
+ ret = target_to_host_drmversion(ver, arg);
+ if (is_error(ret)) {
+ return ret;
+ }
+ ret = get_errno(safe_ioctl(fd, ie->host_cmd, ver));
+ if (is_error(ret)) {
+ unlock_drm_version(ver);
+ return ret;
+ }
+ return host_to_target_drmversion(arg, ver);
+ }
+ return -TARGET_EFAULT;
+}
+
+#endif
+
static IOCTLEntry ioctl_entries[] = {
#define IOCTL(cmd, access, ...) \
{ TARGET_ ## cmd, cmd, #cmd, access, 0, { __VA_ARGS__ } },
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 152ec637cb..3c261cff0e 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1167,6 +1167,9 @@ struct target_rtc_pll_info {
#define TARGET_DM_TARGET_MSG TARGET_IOWRU(0xfd, 0x0e)
#define TARGET_DM_DEV_SET_GEOMETRY TARGET_IOWRU(0xfd, 0x0f)
+/* drm ioctls */
+#define TARGET_DRM_IOCTL_VERSION TARGET_IOWRU('d', 0x00)
+
/* from asm/termbits.h */
#define TARGET_NCC 8
@@ -2598,6 +2601,18 @@ struct target_mq_attr {
abi_long mq_curmsgs;
};
+struct target_drm_version {
+ int version_major;
+ int version_minor;
+ int version_patchlevel;
+ abi_ulong name_len;
+ abi_ulong name;
+ abi_ulong date_len;
+ abi_ulong date;
+ abi_ulong desc_len;
+ abi_ulong desc;
+};
+
#include "socket.h"
#include "errno_defs.h"
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index 4e12c1661e..e2b0484f50 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -292,6 +292,17 @@ STRUCT(dm_target_versions,
STRUCT(dm_target_msg,
TYPE_ULONGLONG) /* sector */
+STRUCT(drm_version,
+ TYPE_INT, /* version_major */
+ TYPE_INT, /* version_minor */
+ TYPE_INT, /* version_patchlevel */
+ TYPE_ULONG, /* name_len */
+ TYPE_PTRVOID, /* name */
+ TYPE_ULONG, /* date_len */
+ TYPE_PTRVOID, /* date */
+ TYPE_ULONG, /* desc_len */
+ TYPE_PTRVOID) /* desc */
+
STRUCT(file_clone_range,
TYPE_LONGLONG, /* src_fd */
TYPE_ULONGLONG, /* src_offset */
--
2.24.0.308.g228f53135a
Le 03/06/2020 à 03:08, chengang@emindsoft.com.cn a écrit :
> From: Chen Gang <chengang@emindsoft.com.cn>
>
> Another DRM_IOCTL_* commands will be done later.
>
> Signed-off-by: Chen Gang <chengang@emindsoft.com.cn>
> ---
> configure | 10 ++++
> linux-user/ioctls.h | 5 ++
> linux-user/syscall.c | 117 +++++++++++++++++++++++++++++++++++++
> linux-user/syscall_defs.h | 15 +++++
> linux-user/syscall_types.h | 11 ++++
> 5 files changed, 158 insertions(+)
>
> diff --git a/configure b/configure
> index e225a1e3ff..3cf28a649a 100755
> --- a/configure
> +++ b/configure
> @@ -3140,6 +3140,13 @@ if ! check_include "ifaddrs.h" ; then
> have_ifaddrs_h=no
> fi
>
> +#########################################
> +# libdrm check
> +have_drm_h=no
> +if check_include "libdrm/drm.h" ; then
> + have_drm_h=yes
> +fi
> +
> ##########################################
> # VTE probe
>
> @@ -7149,6 +7156,9 @@ fi
> if test "$have_ifaddrs_h" = "yes" ; then
> echo "HAVE_IFADDRS_H=y" >> $config_host_mak
> fi
> +if test "$have_drm_h" = "yes" ; then
> + echo "HAVE_DRM_H=y" >> $config_host_mak
> +fi
> if test "$have_broken_size_max" = "yes" ; then
> echo "HAVE_BROKEN_SIZE_MAX=y" >> $config_host_mak
> fi
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 0defa1d8c1..f2e2fa9c87 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -574,6 +574,11 @@
> IOCTL_SPECIAL(SIOCDELRT, IOC_W, do_ioctl_rt,
> MK_PTR(MK_STRUCT(STRUCT_rtentry)))
>
> +#ifdef HAVE_DRM_H
> + IOCTL_SPECIAL(DRM_IOCTL_VERSION, IOC_RW, do_ioctl_drm,
> + MK_PTR(MK_STRUCT(STRUCT_drm_version)))
> +#endif
> +
> #ifdef TARGET_TIOCSTART
> IOCTL_IGNORE(TIOCSTART)
> IOCTL_IGNORE(TIOCSTOP)
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5af55fca78..07b7596e0f 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -112,6 +112,9 @@
> #include <linux/if_alg.h>
> #include <linux/rtc.h>
> #include <sound/asound.h>
> +#ifdef HAVE_DRM_H
> +#include <libdrm/drm.h>
> +#endif
> #include "linux_loop.h"
> #include "uname.h"
>
> @@ -5275,6 +5278,120 @@ static abi_long do_ioctl_tiocgptpeer(const IOCTLEntry *ie, uint8_t *buf_temp,
> }
> #endif
>
> +#ifdef HAVE_DRM_H
> +
> +static void unlock_drm_version(struct drm_version *host_ver)
> +{
> + if (host_ver->name) {
> + unlock_user(host_ver->name, 0UL, 0);
unlock_user() allows to have a NULL host pointer parameter, so you don't
need to check. But you must provide the target pointer, with the length.
The same below.
> + }
> + if (host_ver->date) {
> + unlock_user(host_ver->date, 0UL, 0);
> + }
> + if (host_ver->desc) {
> + unlock_user(host_ver->desc, 0UL, 0);
> + }
> +}
> +
> +static inline abi_long target_to_host_drmversion(struct drm_version *host_ver,
> + abi_long target_addr)
> +{
> + struct target_drm_version *target_ver;
> +
> + if (!lock_user_struct(VERIFY_READ, target_ver, target_addr, 0)) {
> + return -TARGET_EFAULT;
> + }
> +
> + memset(host_ver, 0, sizeof(*host_ver));
> +
> + __get_user(host_ver->name_len, &target_ver->name_len);
> + if (host_ver->name_len) {
> + host_ver->name = lock_user(VERIFY_WRITE, target_ver->name,
> + target_ver->name_len, 0);
> + if (!host_ver->name) {
> + goto err;
> + }
> + }
> +
> + __get_user(host_ver->date_len, &target_ver->date_len);
> + if (host_ver->date_len) {
> + host_ver->date = lock_user(VERIFY_WRITE, target_ver->date,
> + target_ver->date_len, 0);
> + if (!host_ver->date) {
> + goto err;
> + }
> + }
> +
> + __get_user(host_ver->desc_len, &target_ver->desc_len);
> + if (host_ver->desc_len) {
> + host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
> + target_ver->desc_len, 0);
> + if (!host_ver->desc) {
> + goto err;
> + }
> + }
> +
> + unlock_user_struct(target_ver, target_addr, 0);
> + return 0;
> +err:
> + unlock_drm_version(host_ver);
> + unlock_user_struct(target_ver, target_addr, 0);
> + return -ENOMEM;
In fact it should be -TARGET_EFAULT: it has failed because of access rights.
> +}
> +
> +static inline abi_long host_to_target_drmversion(abi_ulong target_addr,
> + struct drm_version *host_ver)
> +{
> + struct target_drm_version *target_ver;
> +
> + if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) {
I think you should not unlock_struct() in target_to_host_drmversion() so
you don't have to lock it again here.
> + unlock_drm_version(host_ver);
> + return -TARGET_EFAULT;
> + }
> + __put_user(host_ver->version_major, &target_ver->version_major);
> + __put_user(host_ver->version_minor, &target_ver->version_minor);
> + __put_user(host_ver->version_patchlevel, &target_ver->version_patchlevel);
> + __put_user(host_ver->name_len, &target_ver->name_len);
> + __put_user(host_ver->date_len, &target_ver->date_len);
> + __put_user(host_ver->desc_len, &target_ver->desc_len);
> + if (host_ver->name) {
> + unlock_user(host_ver->name, target_ver->name, host_ver->name_len);
> + }
No need to check if host_ver->name is NULL.
Same below.
> + if (host_ver->date) {
> + unlock_user(host_ver->date, target_ver->date, host_ver->date_len);
> + }
> + if (host_ver->desc) {
> + unlock_user(host_ver->desc, target_ver->desc, host_ver->desc_len);
> + }
> + unlock_user_struct(target_ver, target_addr, 0);
> + return 0;
> +}
> +
> +static abi_long do_ioctl_drm(const IOCTLEntry *ie, uint8_t *buf_temp,
> + int fd, int cmd, abi_long arg)
> +{
> + struct drm_version *ver;
> + abi_long ret;
> +
> + switch (ie->host_cmd) {
> + case DRM_IOCTL_VERSION:
> + ver = (struct drm_version *)buf_temp;
you should lock the structure here (rather than in
target_to_host_drmversion())...
> + ret = target_to_host_drmversion(ver, arg);
> + if (is_error(ret)) {
> + return ret;
> + }
> + ret = get_errno(safe_ioctl(fd, ie->host_cmd, ver));
> + if (is_error(ret)) {
> + unlock_drm_version(ver);
> + return ret;
> + }
> + return host_to_target_drmversion(arg, ver);
and unlock the structure here (rather than in host_to_target_drmversion()).
You should return "ret" too.
> + }
> + return -TARGET_EFAULT;
Why -TARGET_EFAULT? -TARGET_ENOSYS would be better.
> +}
> +
> +#endif
> +
> static IOCTLEntry ioctl_entries[] = {
> #define IOCTL(cmd, access, ...) \
> { TARGET_ ## cmd, cmd, #cmd, access, 0, { __VA_ARGS__ } },
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 152ec637cb..3c261cff0e 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -1167,6 +1167,9 @@ struct target_rtc_pll_info {
> #define TARGET_DM_TARGET_MSG TARGET_IOWRU(0xfd, 0x0e)
> #define TARGET_DM_DEV_SET_GEOMETRY TARGET_IOWRU(0xfd, 0x0f)
>
> +/* drm ioctls */
> +#define TARGET_DRM_IOCTL_VERSION TARGET_IOWRU('d', 0x00)
> +
> /* from asm/termbits.h */
>
> #define TARGET_NCC 8
> @@ -2598,6 +2601,18 @@ struct target_mq_attr {
> abi_long mq_curmsgs;
> };
>
> +struct target_drm_version {
> + int version_major;
> + int version_minor;
> + int version_patchlevel;
> + abi_ulong name_len;
> + abi_ulong name;
> + abi_ulong date_len;
> + abi_ulong date;
> + abi_ulong desc_len;
> + abi_ulong desc;
> +};
> +
> #include "socket.h"
>
> #include "errno_defs.h"
> diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
> index 4e12c1661e..e2b0484f50 100644
> --- a/linux-user/syscall_types.h
> +++ b/linux-user/syscall_types.h
> @@ -292,6 +292,17 @@ STRUCT(dm_target_versions,
> STRUCT(dm_target_msg,
> TYPE_ULONGLONG) /* sector */
>
> +STRUCT(drm_version,
> + TYPE_INT, /* version_major */
> + TYPE_INT, /* version_minor */
> + TYPE_INT, /* version_patchlevel */
> + TYPE_ULONG, /* name_len */
> + TYPE_PTRVOID, /* name */
> + TYPE_ULONG, /* date_len */
> + TYPE_PTRVOID, /* date */
> + TYPE_ULONG, /* desc_len */
> + TYPE_PTRVOID) /* desc */
> +
> STRUCT(file_clone_range,
> TYPE_LONGLONG, /* src_fd */
> TYPE_ULONGLONG, /* src_offset */
>
Thanks,
Laurent
On 2020/6/3 下午5:49, Laurent Vivier wrote:
> Le 03/06/2020 à 03:08, chengang@emindsoft.com.cn a écrit :
>> +#ifdef HAVE_DRM_H
>> +
>> +static void unlock_drm_version(struct drm_version *host_ver)
>> +{
>> + if (host_ver->name) {
>> + unlock_user(host_ver->name, 0UL, 0);
>
> unlock_user() allows to have a NULL host pointer parameter, so you don't
> need to check. But you must provide the target pointer, with the length.
> The same below.
>
As far as I know, the unlock_user is defined in
include/exec/softmmu-semi.h, which only checks the len before calling
cpu_memory_rw_debug, and only calls free() for the host pointer.
So we have to be sure that the host pointer must be valid. When we pass
0 length to unlock_user, we want it to free host pointer only.
>> + if (host_ver->desc_len) {
>> + host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
>> + target_ver->desc_len, 0);
>> + if (!host_ver->desc) {
>> + goto err;
>> + }
>> + }
>> +
>> + unlock_user_struct(target_ver, target_addr, 0);
>> + return 0;
>> +err:
>> + unlock_drm_version(host_ver);
>> + unlock_user_struct(target_ver, target_addr, 0);
>> + return -ENOMEM;
>
> In fact it should be -TARGET_EFAULT: it has failed because of access rights.
>
As far as I know, the lock_user is defined in
include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case),
lock_user will only malloc a host pointer and return it.
In our case, I guess the only failure from malloc() is "no memory".
>> +}
>>
>> +static inline abi_long host_to_target_drmversion(abi_ulong target_addr,
>> + struct drm_version *host_ver)
>> +{
>> + struct target_drm_version *target_ver;
>> +
>> + if (!lock_user_struct(VERIFY_WRITE, target_ver, target_addr, 0)) {
>
> I think you should not unlock_struct() in target_to_host_drmversion() so
> you don't have to lock it again here.
>
OK, thanks.
>> +static abi_long do_ioctl_drm(const IOCTLEntry *ie, uint8_t *buf_temp,
>> + int fd, int cmd, abi_long arg)
>> +{
>> + struct drm_version *ver;
>> + abi_long ret;
>> +
>> + switch (ie->host_cmd) {
>> + case DRM_IOCTL_VERSION:
>> + ver = (struct drm_version *)buf_temp;
>
> you should lock the structure here (rather than in
> target_to_host_drmversion())...
>
OK, thanks.
>> + ret = target_to_host_drmversion(ver, arg);
>> + if (is_error(ret)) {
>> + return ret;
>> + }
>> + ret = get_errno(safe_ioctl(fd, ie->host_cmd, ver));
>> + if (is_error(ret)) {
>> + unlock_drm_version(ver);
>> + return ret;
>> + }
>> + return host_to_target_drmversion(arg, ver);
>
> and unlock the structure here (rather than in host_to_target_drmversion()).
>
> You should return "ret" too.
>
OK, thanks.
>> + }
>> + return -TARGET_EFAULT;
>
> Why -TARGET_EFAULT? -TARGET_ENOSYS would be better.
>
OK, thanks.
Chen Gang.
Le 03/06/2020 à 13:05, Chen Gang a écrit :
> On 2020/6/3 下午5:49, Laurent Vivier wrote:
>> Le 03/06/2020 à 03:08, chengang@emindsoft.com.cn a écrit :
>>> +#ifdef HAVE_DRM_H
>>> +
>>> +static void unlock_drm_version(struct drm_version *host_ver)
>>> +{
>>> + if (host_ver->name) {
>>> + unlock_user(host_ver->name, 0UL, 0);
>>
>> unlock_user() allows to have a NULL host pointer parameter, so you don't
>> need to check. But you must provide the target pointer, with the length.
>> The same below.
>>
>
> As far as I know, the unlock_user is defined in
> include/exec/softmmu-semi.h, which only checks the len before calling
> cpu_memory_rw_debug, and only calls free() for the host pointer.
>
> So we have to be sure that the host pointer must be valid. When we pass
> 0 length to unlock_user, we want it to free host pointer only.
No, it is defined in our case in linux-user/qemu.h, and associated
comment is:
/* Unlock an area of guest memory. The first LEN bytes must be
flushed back to guest memory. host_ptr = NULL is explicitly
allowed and does nothing. */
>
>>> + if (host_ver->desc_len) {
>>> + host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
>>> + target_ver->desc_len, 0);
>>> + if (!host_ver->desc) {
>>> + goto err;
>>> + }
>>> + }
>>> +
>>> + unlock_user_struct(target_ver, target_addr, 0);
>>> + return 0;
>>> +err:
>>> + unlock_drm_version(host_ver);
>>> + unlock_user_struct(target_ver, target_addr, 0);
>>> + return -ENOMEM;
>>
>> In fact it should be -TARGET_EFAULT: it has failed because of access rights.
>>
>
> As far as I know, the lock_user is defined in
> include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case),
> lock_user will only malloc a host pointer and return it.
No, in linux-user/qemu.h:
/* Lock an area of guest memory into the host. If copy is true then the
host area will have the same contents as the guest. */
> In our case, I guess the only failure from malloc() is "no memory".
See use-cases in syscall.c, they all fail with -TARGET_EFAULT.
Thanks,
Laurent
OK, thanks. I'll send patch v6. :)
On 2020/6/3 下午8:03, Laurent Vivier wrote:
> Le 03/06/2020 à 13:05, Chen Gang a écrit :
>> On 2020/6/3 下午5:49, Laurent Vivier wrote:
>>> Le 03/06/2020 à 03:08, chengang@emindsoft.com.cn a écrit :
>>>> +#ifdef HAVE_DRM_H
>>>> +
>>>> +static void unlock_drm_version(struct drm_version *host_ver)
>>>> +{
>>>> + if (host_ver->name) {
>>>> + unlock_user(host_ver->name, 0UL, 0);
>>>
>>> unlock_user() allows to have a NULL host pointer parameter, so you don't
>>> need to check. But you must provide the target pointer, with the length.
>>> The same below.
>>>
>>
>> As far as I know, the unlock_user is defined in
>> include/exec/softmmu-semi.h, which only checks the len before calling
>> cpu_memory_rw_debug, and only calls free() for the host pointer.
>>
>> So we have to be sure that the host pointer must be valid. When we pass
>> 0 length to unlock_user, we want it to free host pointer only.
>
> No, it is defined in our case in linux-user/qemu.h, and associated
> comment is:
>
> /* Unlock an area of guest memory. The first LEN bytes must be
> flushed back to guest memory. host_ptr = NULL is explicitly
> allowed and does nothing. */
>
>>
>>>> + if (host_ver->desc_len) {
>>>> + host_ver->desc = lock_user(VERIFY_WRITE, target_ver->desc,
>>>> + target_ver->desc_len, 0);
>>>> + if (!host_ver->desc) {
>>>> + goto err;
>>>> + }
>>>> + }
>>>> +
>>>> + unlock_user_struct(target_ver, target_addr, 0);
>>>> + return 0;
>>>> +err:
>>>> + unlock_drm_version(host_ver);
>>>> + unlock_user_struct(target_ver, target_addr, 0);
>>>> + return -ENOMEM;
>>>
>>> In fact it should be -TARGET_EFAULT: it has failed because of access rights.
>>>
>>
>> As far as I know, the lock_user is defined in
>> include/exec/softmmu-semi.h. If the parameter 'copy' is 0 (in our case),
>> lock_user will only malloc a host pointer and return it.
>
> No, in linux-user/qemu.h:
>
> /* Lock an area of guest memory into the host. If copy is true then the
> host area will have the same contents as the guest. */
>
>> In our case, I guess the only failure from malloc() is "no memory".
>
> See use-cases in syscall.c, they all fail with -TARGET_EFAULT.
© 2016 - 2024 Red Hat, Inc.