qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] linux-user: rt_sigprocmask, check read perms first


From: Patrick Venture
Subject: Re: [PATCH] linux-user: rt_sigprocmask, check read perms first
Date: Tue, 11 Jan 2022 12:14:37 -0800



On Sat, Jan 8, 2022 at 10:16 AM Laurent Vivier <laurent@vivier.eu> wrote:
Le 06/01/2022 à 23:00, Patrick Venture a écrit :
> From: Shu-Chun Weng <scw@google.com>
>
> Linux kernel does it this way (checks read permission before validating `how`)
> and the latest version of ABSL's `AddressIsReadable()` depends on this
> behavior.
>
> c.f.  https://github.com/torvalds/linux/blob/9539ba4308ad5bdca6cb41c7b73cbb9f796dcdd7/kernel/signal.c#L3147
> Reviewed-by: Patrick Venture <venture@google.com>
> Signed-off-by: Shu-Chun Weng <scw@google.com>
> ---
>   linux-user/syscall.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index ce9d64896c..3070d31f34 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9491,6 +9491,11 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>               }
>   
>               if (arg2) {
> +                if (!(p = lock_user(VERIFY_READ, arg2, sizeof(target_sigset_t), 1)))
> +                    return -TARGET_EFAULT;
> +                target_to_host_sigset(&set, p);
> +                unlock_user(p, arg2, 0);
> +                set_ptr = &set;
>                   switch(how) {
>                   case TARGET_SIG_BLOCK:
>                       how = SIG_BLOCK;
> @@ -9504,11 +9509,6 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>                   default:
>                       return -TARGET_EINVAL;
>                   }
> -                if (!(p = lock_user(VERIFY_READ, arg2, sizeof(target_sigset_t), 1)))
> -                    return -TARGET_EFAULT;
> -                target_to_host_sigset(&set, p);
> -                unlock_user(p, arg2, 0);
> -                set_ptr = &set;
>               } else {
>                   how = 0;
>                   set_ptr = NULL;

I know it's only code move but generally we also update the style to pass scripts/checkpatch.pl
successfully.

That is a reasonable request, however, can I just send a follow-on patch?  I didn't write this one and I honestly don't know much about it, but I don't mind doing the cleanup
 

Could you also update TARGET_NR_sigprocmask in the same way as it seems the kernel behaves like this
too in this case?

I can take a look.  I would prefer then to also prefetch the style fixup in a preceding patch. I don't recall seeing whether qemu supports clang-format.
 

Thanks,
Laurent

Patrick 

reply via email to

[Prev in Thread] Current Thread [Next in Thread]