Re: [PATCH v3 17/23] bsd-user: Implement mincore(2)

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

On 9/9/23 12:36, Karim Taha wrote:
> From: Stacey Son <sson@FreeBSD.org>
> 
> Signed-off-by: Stacey Son <sson@FreeBSD.org>
> Signed-off-by: Karim Taha <kariem.taha2.7@gmail.com>
> ---
>   bsd-user/bsd-mem.h            | 22 ++++++++++++++++++++++
>   bsd-user/freebsd/os-syscall.c |  4 ++++
>   2 files changed, 26 insertions(+)
> 
> diff --git a/bsd-user/bsd-mem.h b/bsd-user/bsd-mem.h
> index 0e16051418..1dabbe36e6 100644
> --- a/bsd-user/bsd-mem.h
> +++ b/bsd-user/bsd-mem.h
> @@ -189,4 +189,26 @@ static inline abi_long do_bsd_minherit(abi_long addr, abi_long len,
>       return get_errno(minherit(g2h_untagged(addr), len, inherit));
>   }
>   
> +/* mincore(2) */
> +static inline abi_long do_bsd_mincore(abi_ulong target_addr, abi_ulong len,
> +        abi_ulong target_vec)
> +{
> +    abi_long ret;
> +    void *p;
> +    abi_ulong vec_len = DIV_ROUND_UP(len,TARGET_PAGE_SIZE);
> +
> +    if (!guest_range_valid_untagged(target_addr,len) || !page_check_range(target_addr, len, PAGE_VALID)) {
> +        return -TARGET_EFAULT;
> +    }
> +
> +    p = lock_user(VERIFY_WRITE, target_vec, vec_len, 0);
> +    if (p == NULL) {
> +        return -TARGET_EFAULT;
> +    }
> +    ret = get_errno(mincore(g2h_untagged(target_addr), len, p));
> +    unlock_user(p, target_vec, 0);

You don't need the lock/unlock_user at all.  It is wrongly checking for WRITE.


r~

> +
> +    return ret;
> +}
> +
>   #endif /* BSD_USER_BSD_MEM_H */
> diff --git a/bsd-user/freebsd/os-syscall.c b/bsd-user/freebsd/os-syscall.c
> index f5d60cf902..8d1cf3b35c 100644
> --- a/bsd-user/freebsd/os-syscall.c
> +++ b/bsd-user/freebsd/os-syscall.c
> @@ -527,6 +527,10 @@ static abi_long freebsd_syscall(void *cpu_env, int num, abi_long arg1,
>           ret = do_bsd_minherit(arg1, arg2, arg3);
>           break;
>   
> +    case TARGET_FREEBSD_NR_mincore: /* mincore(2) */
> +        ret = do_bsd_mincore(arg1, arg2, arg3);
> +        break;
> +
>   #if defined(__FreeBSD_version) && __FreeBSD_version >= 1300048
>       case TARGET_FREEBSD_NR_shm_open2: /* shm_open2(2) */
>           ret = do_freebsd_shm_open2(arg1, arg2, arg3, arg4, arg5);