| Message ID | 20220126175850.1904968-2-venture@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user: check read permissions before how | expand |
Le 26/01/2022 à 18:58, 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> Reviewed-by: Laurent Vivier <laurent@vivier.eu> but you must resend the patch: you are not the author, but you have to add your Signed-off-by. (and now you can add my reviewed-by) https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n296 Thanks, Laurent > --- > linux-user/syscall.c | 12 +++++++----- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 5950222a77..34bd819e38 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -9508,6 +9508,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, > } > > if (arg2) { > + p = lock_user(VERIFY_READ, arg2, sizeof(target_sigset_t), 1); > + if (!p) { > + 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; > @@ -9521,11 +9528,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;
On Wed, Jan 26, 2022 at 11:26 AM Laurent Vivier <laurent@vivier.eu> wrote: > Le 26/01/2022 à 18:58, 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> > > Reviewed-by: Laurent Vivier <laurent@vivier.eu> > > but you must resend the patch: you are not the author, but you have to add > your Signed-off-by. > (and now you can add my reviewed-by) > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/SubmittingPatches?id=f6f94e2ab1b33f0082ac22d71f66385a60d8157f#n296 Thanks! I definitely forgot to sign the patches I wasn't the author -- but you're right and thanks for pointing me to the guide. > > > Thanks, > Laurent > > > --- > > linux-user/syscall.c | 12 +++++++----- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 5950222a77..34bd819e38 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -9508,6 +9508,13 @@ static abi_long do_syscall1(void *cpu_env, int > num, abi_long arg1, > > } > > > > if (arg2) { > > + p = lock_user(VERIFY_READ, arg2, > sizeof(target_sigset_t), 1); > > + if (!p) { > > + 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; > > @@ -9521,11 +9528,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; > >
diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 5950222a77..34bd819e38 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9508,6 +9508,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, } if (arg2) { + p = lock_user(VERIFY_READ, arg2, sizeof(target_sigset_t), 1); + if (!p) { + 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; @@ -9521,11 +9528,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;