Hi Patrick,
Le 11/01/2022 à 21:14, Patrick Venture a écrit :
>
>
> On Sat, Jan 8, 2022 at 10:16 AM Laurent Vivier <laurent@vivier.eu <mailto:laurent@vivier.eu>> wrote:
>
> Le 06/01/2022 à 23:00, Patrick Venture a écrit :
> > From: Shu-Chun Weng <scw@google.com <mailto: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
> <https://github.com/torvalds/linux/blob/9539ba4308ad5bdca6cb41c7b73cbb9f796dcdd7/kernel/signal.c#L3147>
> > Reviewed-by: Patrick Venture <venture@google.com <mailto:venture@google.com>>
> > Signed-off-by: Shu-Chun Weng <scw@google.com <mailto: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
> <http://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.
>
There is no problem. You can keep this patch unmodified, and add patches to fix the problems.
I only ask to have all the patches in one series.
Will take a swing at this for v2.
Thanks,
Laurent