Richard Henderson <[email protected]> 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));
__put_user(host_sd->shm_segsz, &target_sd->shm_segsz);
__put_user(host_sd->shm_lpid, &target_sd->shm_lpid);
__put_user(host_sd->shm_cpid, &target_sd->shm_cpid);
__put_user(host_sd->shm_nattch, &target_sd->shm_nattch);
__put_user(host_sd->shm_atime, &target_sd->shm_atime);
__put_user(host_sd->shm_dtime, &target_sd->shm_dtime);
__put_user(host_sd->shm_ctime, &target_sd->shm_ctime);
unlock_user_struct(target_sd, target_addr, 1);
return 0;
}
As far as I understood the `page_check_range` function, defined at
accel/tcg/user-exec.c::523:
-The locked range is (target_addr, target_addr + sizeof(target_ipc_perm) -1) in
case of
host_to_target_ipc_perm function.
-The locked range is (target_addr, taregt_addr + sizeof(target_shmid_ds) -1) in
case of
host_to_target_shmid_ds function.
Since `host_to_target_shmid_ds` struct has larger size, in the original
code, is the sucess of the first lock guarantees the sucess of the
second?
If I got it wrong, please elaborate further.
If I'm correct, do you think I should call g2h_untagged in
`host_to_target_ipc_perm__locked` directly, or should I receive it as a
paremeter?
--
Kariiem Taha
