[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION
From: |
Chen Gang |
Subject: |
Re: [PATCH v5] linux-user: syscall: ioctls: support DRM_IOCTL_VERSION |
Date: |
Thu, 4 Jun 2020 09:04:40 +0800 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
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.