* [PATCH v2 0/2] linux-user: check read permissions before how
@ 2022-01-26 17:58 Patrick Venture
2022-01-26 17:58 ` [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first Patrick Venture
2022-01-26 17:58 ` [PATCH v2 2/2] linux-user: sigprocmask " Patrick Venture
0 siblings, 2 replies; 6+ messages in thread
From: Patrick Venture @ 2022-01-26 17:58 UTC (permalink / raw)
To: laurent; +Cc: qemu-devel, scw, Patrick Venture
In sigprocmask check the read permissions first before checking the `how`.
This is done for both: TARGET_NR_sigprocmask and TARGET_NR_rt_sigprocmask
v2:
* Update code style during code change
* Also update check order for TARGET_NR_sigprocmask
Patrick Venture (1):
linux-user: sigprocmask check read perms first
Shu-Chun Weng (1):
linux-user: rt_sigprocmask, check read perms first
linux-user/syscall.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first
2022-01-26 17:58 [PATCH v2 0/2] linux-user: check read permissions before how Patrick Venture
@ 2022-01-26 17:58 ` Patrick Venture
2022-01-26 19:26 ` Laurent Vivier
2022-01-26 17:58 ` [PATCH v2 2/2] linux-user: sigprocmask " Patrick Venture
1 sibling, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2022-01-26 17:58 UTC (permalink / raw)
To: laurent; +Cc: qemu-devel, scw, Patrick Venture
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 | 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;
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] linux-user: sigprocmask check read perms first
2022-01-26 17:58 [PATCH v2 0/2] linux-user: check read permissions before how Patrick Venture
2022-01-26 17:58 ` [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first Patrick Venture
@ 2022-01-26 17:58 ` Patrick Venture
2022-01-26 19:26 ` Laurent Vivier
1 sibling, 1 reply; 6+ messages in thread
From: Patrick Venture @ 2022-01-26 17:58 UTC (permalink / raw)
To: laurent; +Cc: qemu-devel, scw, Patrick Venture
Linux kernel now checks the read permissions before validating `how`
Suggested-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Patrick Venture <venture@google.com>
---
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 34bd819e38..210483d4e4 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -9465,6 +9465,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
int how;
if (arg2) {
+ p = lock_user(VERIFY_READ, arg2, sizeof(target_sigset_t), 1));
+ if (!p) {
+ return -TARGET_EFAULT;
+ }
+ target_to_host_old_sigset(&set, p);
+ unlock_user(p, arg2, 0);
+ set_ptr = &set;
switch (arg1) {
case TARGET_SIG_BLOCK:
how = SIG_BLOCK;
@@ -9478,11 +9485,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_old_sigset(&set, p);
- unlock_user(p, arg2, 0);
- set_ptr = &set;
} else {
how = 0;
set_ptr = NULL;
--
2.35.0.rc0.227.g00780c9af4-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first
2022-01-26 17:58 ` [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first Patrick Venture
@ 2022-01-26 19:26 ` Laurent Vivier
2022-01-26 21:10 ` Patrick Venture
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Vivier @ 2022-01-26 19:26 UTC (permalink / raw)
To: Patrick Venture; +Cc: qemu-devel, scw
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;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] linux-user: sigprocmask check read perms first
2022-01-26 17:58 ` [PATCH v2 2/2] linux-user: sigprocmask " Patrick Venture
@ 2022-01-26 19:26 ` Laurent Vivier
0 siblings, 0 replies; 6+ messages in thread
From: Laurent Vivier @ 2022-01-26 19:26 UTC (permalink / raw)
To: Patrick Venture; +Cc: qemu-devel, scw
Le 26/01/2022 à 18:58, Patrick Venture a écrit :
> Linux kernel now checks the read permissions before validating `how`
>
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Patrick Venture <venture@google.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> 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 34bd819e38..210483d4e4 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -9465,6 +9465,13 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> int how;
>
> if (arg2) {
> + p = lock_user(VERIFY_READ, arg2, sizeof(target_sigset_t), 1));
> + if (!p) {
> + return -TARGET_EFAULT;
> + }
> + target_to_host_old_sigset(&set, p);
> + unlock_user(p, arg2, 0);
> + set_ptr = &set;
> switch (arg1) {
> case TARGET_SIG_BLOCK:
> how = SIG_BLOCK;
> @@ -9478,11 +9485,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_old_sigset(&set, p);
> - unlock_user(p, arg2, 0);
> - set_ptr = &set;
> } else {
> how = 0;
> set_ptr = NULL;
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first
2022-01-26 19:26 ` Laurent Vivier
@ 2022-01-26 21:10 ` Patrick Venture
0 siblings, 0 replies; 6+ messages in thread
From: Patrick Venture @ 2022-01-26 21:10 UTC (permalink / raw)
To: Laurent Vivier; +Cc: QEMU Developers, Shu-Chun Weng
[-- Attachment #1: Type: text/plain, Size: 2600 bytes --]
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;
>
>
[-- Attachment #2: Type: text/html, Size: 4120 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-01-26 21:11 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-26 17:58 [PATCH v2 0/2] linux-user: check read permissions before how Patrick Venture
2022-01-26 17:58 ` [PATCH v2 1/2] linux-user: rt_sigprocmask, check read perms first Patrick Venture
2022-01-26 19:26 ` Laurent Vivier
2022-01-26 21:10 ` Patrick Venture
2022-01-26 17:58 ` [PATCH v2 2/2] linux-user: sigprocmask " Patrick Venture
2022-01-26 19:26 ` Laurent Vivier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.