Re: [PATCH 10/22] Implement shmid_ds conversion between host and target.

[Richard Henderson <richard.henderson@linaro.org>]

On 9/3/23 01:45, Kariiem Taha wrote:
> Richard Henderson <richard.henderson@linaro.org> writes:
> 
>> On 8/19/23 02:47, Karim Taha wrote:
>>> +    if (!lock_user_struct(VERIFY_WRITE, target_sd, target_addr, 0)) {
>>> +        return -TARGET_EFAULT;
>>> +    }
>>> +    if (host_to_target_ipc_perm(target_addr, &(host_sd->shm_perm))) {
>>> +        return -TARGET_EFAULT;
>>> +    }
>>
>> While it works, ideally you wouldn't double-lock a memory range, once here and once in
>> host_to_target_ipc_perm.  You could split out the middle of the function as
>> host_to_target_ipc_perm__locked.
> 
> Hi Richard,
> 
> Can you please verify the correctness of the following refactoring?
>      void host_to_target_ipc_perm__locked(abi_ulong target_addr,
>              struct ipc_perm *host_ip)
>      {
>          struct target_ipc_perm *target_ip = g2h_untagged(target_addr);
>          __put_user(host_ip->cuid, &target_ip->cuid);
>          __put_user(host_ip->cgid, &target_ip->cgid);
>          __put_user(host_ip->uid, &target_ip->uid);
>          __put_user(host_ip->gid, &target_ip->gid);
>          __put_user(host_ip->mode, &target_ip->mode);
>          __put_user(host_ip->seq, &target_ip->seq);
>          __put_user(host_ip->key, &target_ip->key);
>      }
> 
>      abi_long host_to_target_shmid_ds(abi_ulong target_addr,
>              struct shmid_ds *host_sd)
>      {
>          struct target_shmid_ds *target_sd;
>          target_sd = lock_user(VERIFY_WRITE, target_addr, sizeof(*target_sd), 0);
>          if (!target_sd){
>              return -TARGET_EFAULT;
>          }
> 
>          host_to_target_ipc_perm__locked(target_addr, &(host_sd->shm_perm));

No.  You'd pass &target_sd->shm_perm, not target_addr, and you don't use g2h at all.


r~