2021-04-13 09:26:56

by Eric Dumazet

Subject: [PATCH 2/3] rseq: remove redundant access_ok()

From: Eric Dumazet <[email protected]>

After commit 8f2817701492 ("rseq: Use get_user/put_user rather
than __get_user/__put_user") we no longer need
an access_ok() call from __rseq_handle_notify_resume()

Signed-off-by: Eric Dumazet <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: "Paul E. McKenney" <[email protected]>
Cc: Boqun Feng <[email protected]>
Cc: Arjun Roy <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
kernel/rseq.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/kernel/rseq.c b/kernel/rseq.c
index d2689ccbb132c0fc8ec0924008771e5ee1ca855e..57344f9abb43905c7dd2b6081205ff508d963e1e 100644
--- a/kernel/rseq.c
+++ b/kernel/rseq.c
@@ -273,8 +273,6 @@ void __rseq_handle_notify_resume(struct ksignal *ksig, struct pt_regs *regs)

if (unlikely(t->flags & PF_EXITING))
return;
- if (unlikely(!access_ok(t->rseq, sizeof(*t->rseq))))
- goto error;
ret = rseq_ip_fixup(regs);
if (unlikely(ret < 0))
goto error;
--
2.31.1.295.g9ea45b61b8-goog

2021-04-13 19:05:13

by Mathieu Desnoyers

Subject: Re: [PATCH 2/3] rseq: remove redundant access_ok()

----- On Apr 13, 2021, at 3:36 AM, Eric Dumazet [email protected] wrote:

> From: Eric Dumazet <[email protected]>
>
> After commit 8f2817701492 ("rseq: Use get_user/put_user rather
> than __get_user/__put_user") we no longer need
> an access_ok() call from __rseq_handle_notify_resume()

While we are doing that, should we also remove the access_ok() check in
rseq_syscall() ? It look like it can also be removed for the exact same
reason outlined here.

Thanks,

Mathieu

>
> Signed-off-by: Eric Dumazet <[email protected]>
> Cc: Mathieu Desnoyers <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: "Paul E. McKenney" <[email protected]>
> Cc: Boqun Feng <[email protected]>
> Cc: Arjun Roy <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> ---
> kernel/rseq.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/kernel/rseq.c b/kernel/rseq.c
> index
> d2689ccbb132c0fc8ec0924008771e5ee1ca855e..57344f9abb43905c7dd2b6081205ff508d963e1e
> 100644
> --- a/kernel/rseq.c
> +++ b/kernel/rseq.c
> @@ -273,8 +273,6 @@ void __rseq_handle_notify_resume(struct ksignal *ksig,
> struct pt_regs *regs)
>
> if (unlikely(t->flags & PF_EXITING))
> return;
> - if (unlikely(!access_ok(t->rseq, sizeof(*t->rseq))))
> - goto error;
> ret = rseq_ip_fixup(regs);
> if (unlikely(ret < 0))
> goto error;
> --
> 2.31.1.295.g9ea45b61b8-goog

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

2021-04-14 01:22:40

by Eric Dumazet

Subject: Re: [PATCH 2/3] rseq: remove redundant access_ok()

On Tue, Apr 13, 2021 at 4:34 PM Mathieu Desnoyers
<[email protected]> wrote:
>
> ----- On Apr 13, 2021, at 3:36 AM, Eric Dumazet [email protected] wrote:
>
> > From: Eric Dumazet <[email protected]>
> >
> > After commit 8f2817701492 ("rseq: Use get_user/put_user rather
> > than __get_user/__put_user") we no longer need
> > an access_ok() call from __rseq_handle_notify_resume()
>
> While we are doing that, should we also remove the access_ok() check in
> rseq_syscall() ? It look like it can also be removed for the exact same
> reason outlined here.

Yes, good idea.

I was focusing in __rseq_handle_notify_resume() paths but
rseq_syscall() can get the same.

> Thanks,
>
> Mathieu
>
> >
> > Signed-off-by: Eric Dumazet <[email protected]>
> > Cc: Mathieu Desnoyers <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: "Paul E. McKenney" <[email protected]>
> > Cc: Boqun Feng <[email protected]>
> > Cc: Arjun Roy <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > ---
> > kernel/rseq.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/kernel/rseq.c b/kernel/rseq.c
> > index
> > d2689ccbb132c0fc8ec0924008771e5ee1ca855e..57344f9abb43905c7dd2b6081205ff508d963e1e
> > 100644
> > --- a/kernel/rseq.c
> > +++ b/kernel/rseq.c
> > @@ -273,8 +273,6 @@ void __rseq_handle_notify_resume(struct ksignal *ksig,
> > struct pt_regs *regs)
> >
> > if (unlikely(t->flags & PF_EXITING))
> > return;
> > - if (unlikely(!access_ok(t->rseq, sizeof(*t->rseq))))
> > - goto error;
> > ret = rseq_ip_fixup(regs);
> > if (unlikely(ret < 0))
> > goto error;
> > --
> > 2.31.1.295.g9ea45b61b8-goog
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com