* [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
@ 2019-03-29 17:12 Dmitry V. Levin
2019-03-29 17:15 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Dmitry V. Levin @ 2019-03-29 17:12 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: Will Drewry, Kees Cook, linux-kernel, Steven Rostedt,
Andy Lutomirski, Ingo Molnar, linux-riscv
RISC-V syscall arguments are located in orig_a0,a1..a5 fields
of struct pt_regs.
Due to an off-by-one bug and a bug in pointer arithmetic
syscall_get_arguments() was reading s3..s7 fields instead of a1..a5.
Likewise, syscall_set_arguments() was writing s3..s7 fields
instead of a1..a5.
Fixes: e2c0cdfba7f69 ("RISC-V: User-facing API")
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Will Drewry <wad@chromium.org>
Cc: linux-riscv@lists.infradead.org
Cc: stable@vger.kernel.org # v4.15+
Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
arch/riscv/include/asm/syscall.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index bba3da6ef157..6ea9e1804233 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -79,10 +79,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
if (i == 0) {
args[0] = regs->orig_a0;
args++;
- i++;
n--;
+ } else {
+ i--;
}
- memcpy(args, ®s->a1 + i * sizeof(regs->a1), n * sizeof(args[0]));
+ memcpy(args, ®s->a1 + i, n * sizeof(args[0]));
}
static inline void syscall_set_arguments(struct task_struct *task,
@@ -94,10 +95,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
if (i == 0) {
regs->orig_a0 = args[0];
args++;
- i++;
n--;
- }
- memcpy(®s->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0));
+ } else {
+ i--;
+ }
+ memcpy(®s->a1 + i, args, n * sizeof(regs->a1));
}
static inline int syscall_get_arch(void)
--
ldv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 17:12 [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments() Dmitry V. Levin
@ 2019-03-29 17:15 ` Steven Rostedt
2019-03-29 17:52 ` David Abdurachmanov
2019-03-29 18:16 ` Dmitry V. Levin
0 siblings, 2 replies; 9+ messages in thread
From: Steven Rostedt @ 2019-03-29 17:15 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Will Drewry, Kees Cook, Palmer Dabbelt, linux-kernel,
Andy Lutomirski, Ingo Molnar, linux-riscv
On Fri, 29 Mar 2019 20:12:21 +0300
"Dmitry V. Levin" <ldv@altlinux.org> wrote:
> RISC-V syscall arguments are located in orig_a0,a1..a5 fields
> of struct pt_regs.
>
> Due to an off-by-one bug and a bug in pointer arithmetic
> syscall_get_arguments() was reading s3..s7 fields instead of a1..a5.
> Likewise, syscall_set_arguments() was writing s3..s7 fields
> instead of a1..a5.
Should I add this to my series? And then rebase on top of it?
-- Steve
>
> Fixes: e2c0cdfba7f69 ("RISC-V: User-facing API")
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Will Drewry <wad@chromium.org>
> Cc: linux-riscv@lists.infradead.org
> Cc: stable@vger.kernel.org # v4.15+
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
> arch/riscv/include/asm/syscall.h | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> index bba3da6ef157..6ea9e1804233 100644
> --- a/arch/riscv/include/asm/syscall.h
> +++ b/arch/riscv/include/asm/syscall.h
> @@ -79,10 +79,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> if (i == 0) {
> args[0] = regs->orig_a0;
> args++;
> - i++;
> n--;
> + } else {
> + i--;
> }
> - memcpy(args, ®s->a1 + i * sizeof(regs->a1), n * sizeof(args[0]));
> + memcpy(args, ®s->a1 + i, n * sizeof(args[0]));
> }
>
> static inline void syscall_set_arguments(struct task_struct *task,
> @@ -94,10 +95,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
> if (i == 0) {
> regs->orig_a0 = args[0];
> args++;
> - i++;
> n--;
> - }
> - memcpy(®s->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0));
> + } else {
> + i--;
> + }
> + memcpy(®s->a1 + i, args, n * sizeof(regs->a1));
> }
>
> static inline int syscall_get_arch(void)
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 17:15 ` Steven Rostedt
@ 2019-03-29 17:52 ` David Abdurachmanov
2019-03-29 17:56 ` Steven Rostedt
2019-03-29 18:16 ` Dmitry V. Levin
1 sibling, 1 reply; 9+ messages in thread
From: David Abdurachmanov @ 2019-03-29 17:52 UTC (permalink / raw)
To: Steven Rostedt
Cc: Will Drewry, Kees Cook, Palmer Dabbelt, linux-kernel,
Andy Lutomirski, Ingo Molnar, linux-riscv, Dmitry V. Levin
On Fri, Mar 29, 2019 at 6:15 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 29 Mar 2019 20:12:21 +0300
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>
> > RISC-V syscall arguments are located in orig_a0,a1..a5 fields
> > of struct pt_regs.
> >
> > Due to an off-by-one bug and a bug in pointer arithmetic
> > syscall_get_arguments() was reading s3..s7 fields instead of a1..a5.
> > Likewise, syscall_set_arguments() was writing s3..s7 fields
> > instead of a1..a5.
>
> Should I add this to my series? And then rebase on top of it?
I have alternative version posted in December part of SECCOMP
patchset which is based on arm64 implementation.
http://lists.infradead.org/pipermail/linux-riscv/2018-December/002450.html
I noticed that SECCOMP wasn't working properly if filters were
checking syscall arguments, because populated arguments were wrong.
Btw, I plan to send v2 of SECCOMP patchset soonish.
david
>
> -- Steve
>
> >
> > Fixes: e2c0cdfba7f69 ("RISC-V: User-facing API")
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Kees Cook <keescook@chromium.org>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Will Drewry <wad@chromium.org>
> > Cc: linux-riscv@lists.infradead.org
> > Cc: stable@vger.kernel.org # v4.15+
> > Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> > ---
> > arch/riscv/include/asm/syscall.h | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
> > index bba3da6ef157..6ea9e1804233 100644
> > --- a/arch/riscv/include/asm/syscall.h
> > +++ b/arch/riscv/include/asm/syscall.h
> > @@ -79,10 +79,11 @@ static inline void syscall_get_arguments(struct task_struct *task,
> > if (i == 0) {
> > args[0] = regs->orig_a0;
> > args++;
> > - i++;
> > n--;
> > + } else {
> > + i--;
> > }
> > - memcpy(args, ®s->a1 + i * sizeof(regs->a1), n * sizeof(args[0]));
> > + memcpy(args, ®s->a1 + i, n * sizeof(args[0]));
> > }
> >
> > static inline void syscall_set_arguments(struct task_struct *task,
> > @@ -94,10 +95,11 @@ static inline void syscall_set_arguments(struct task_struct *task,
> > if (i == 0) {
> > regs->orig_a0 = args[0];
> > args++;
> > - i++;
> > n--;
> > - }
> > - memcpy(®s->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0));
> > + } else {
> > + i--;
> > + }
> > + memcpy(®s->a1 + i, args, n * sizeof(regs->a1));
> > }
> >
> > static inline int syscall_get_arch(void)
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 17:52 ` David Abdurachmanov
@ 2019-03-29 17:56 ` Steven Rostedt
2019-03-29 18:11 ` Dmitry V. Levin
0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2019-03-29 17:56 UTC (permalink / raw)
To: David Abdurachmanov
Cc: Will Drewry, Kees Cook, Palmer Dabbelt, linux-kernel,
Andy Lutomirski, Ingo Molnar, linux-riscv, Dmitry V. Levin
On Fri, 29 Mar 2019 18:52:18 +0100
David Abdurachmanov <david.abdurachmanov@gmail.com> wrote:
> I have alternative version posted in December part of SECCOMP
> patchset which is based on arm64 implementation.
>
> http://lists.infradead.org/pipermail/linux-riscv/2018-December/002450.html
>
> I noticed that SECCOMP wasn't working properly if filters were
> checking syscall arguments, because populated arguments were wrong.
>
> Btw, I plan to send v2 of SECCOMP patchset soonish.
Please do. I want to get my patch series out, which will require these
changes.
-- Steve
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 17:56 ` Steven Rostedt
@ 2019-03-29 18:11 ` Dmitry V. Levin
2019-03-29 20:32 ` Steven Rostedt
0 siblings, 1 reply; 9+ messages in thread
From: Dmitry V. Levin @ 2019-03-29 18:11 UTC (permalink / raw)
To: Steven Rostedt
Cc: Will Drewry, Kees Cook, David Abdurachmanov, Palmer Dabbelt,
linux-kernel, Andy Lutomirski, Ingo Molnar, linux-riscv
[-- Attachment #1.1: Type: text/plain, Size: 1092 bytes --]
On Fri, Mar 29, 2019 at 01:56:35PM -0400, Steven Rostedt wrote:
> On Fri, 29 Mar 2019 18:52:18 +0100
> David Abdurachmanov <david.abdurachmanov@gmail.com> wrote:
>
> > I have alternative version posted in December part of SECCOMP
> > patchset which is based on arm64 implementation.
> >
> > http://lists.infradead.org/pipermail/linux-riscv/2018-December/002450.html
> >
> > I noticed that SECCOMP wasn't working properly if filters were
> > checking syscall arguments, because populated arguments were wrong.
> >
> > Btw, I plan to send v2 of SECCOMP patchset soonish.
>
> Please do. I want to get my patch series out, which will require these
> changes.
Sorry, I haven't seen the alternative patch posted by David before.
Apparently, besides fixing the bug it also introduces new sanity checks
of "i" and "n" arguments in syscall_get_arguments() and
syscall_set_arguments().
Given that your patchset removes these arguments completely,
I see little sense in adding new checks that are going to be removed
by the subsequent commit in the series.
--
ldv
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 18:11 ` Dmitry V. Levin
@ 2019-03-29 20:32 ` Steven Rostedt
0 siblings, 0 replies; 9+ messages in thread
From: Steven Rostedt @ 2019-03-29 20:32 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Will Drewry, Kees Cook, David Abdurachmanov, Palmer Dabbelt,
linux-kernel, Andy Lutomirski, Ingo Molnar, linux-riscv
On Fri, 29 Mar 2019 21:11:09 +0300
"Dmitry V. Levin" <ldv@altlinux.org> wrote:
> On Fri, Mar 29, 2019 at 01:56:35PM -0400, Steven Rostedt wrote:
> > On Fri, 29 Mar 2019 18:52:18 +0100
> > David Abdurachmanov <david.abdurachmanov@gmail.com> wrote:
> >
> > > I have alternative version posted in December part of SECCOMP
> > > patchset which is based on arm64 implementation.
> > >
> > > http://lists.infradead.org/pipermail/linux-riscv/2018-December/002450.html
> > >
> > > I noticed that SECCOMP wasn't working properly if filters were
> > > checking syscall arguments, because populated arguments were wrong.
> > >
> > > Btw, I plan to send v2 of SECCOMP patchset soonish.
> >
> > Please do. I want to get my patch series out, which will require these
> > changes.
>
> Sorry, I haven't seen the alternative patch posted by David before.
> Apparently, besides fixing the bug it also introduces new sanity checks
> of "i" and "n" arguments in syscall_get_arguments() and
> syscall_set_arguments().
>
> Given that your patchset removes these arguments completely,
> I see little sense in adding new checks that are going to be removed
> by the subsequent commit in the series.
I agree. I'm going to pull in Dmitry's patches as my patches are going
to rewrite most the code anyway, and no need for the extra churn of the
sanity checks that are going to become irrelevant immediately afterward.
-- Steve
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 17:15 ` Steven Rostedt
2019-03-29 17:52 ` David Abdurachmanov
@ 2019-03-29 18:16 ` Dmitry V. Levin
2019-03-29 20:33 ` Steven Rostedt
1 sibling, 1 reply; 9+ messages in thread
From: Dmitry V. Levin @ 2019-03-29 18:16 UTC (permalink / raw)
To: Steven Rostedt
Cc: Will Drewry, Kees Cook, Palmer Dabbelt, linux-kernel,
Andy Lutomirski, Ingo Molnar, Guo Ren, linux-riscv
[-- Attachment #1.1: Type: text/plain, Size: 669 bytes --]
On Fri, Mar 29, 2019 at 01:15:14PM -0400, Steven Rostedt wrote:
> On Fri, 29 Mar 2019 20:12:21 +0300
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>
> > RISC-V syscall arguments are located in orig_a0,a1..a5 fields
> > of struct pt_regs.
> >
> > Due to an off-by-one bug and a bug in pointer arithmetic
> > syscall_get_arguments() was reading s3..s7 fields instead of a1..a5.
> > Likewise, syscall_set_arguments() was writing s3..s7 fields
> > instead of a1..a5.
>
> Should I add this to my series? And then rebase on top of it?
This is fine with me. If you are adding the fix for riscv,
please consider adding the fix for csky, too.
--
ldv
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 161 bytes --]
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 18:16 ` Dmitry V. Levin
@ 2019-03-29 20:33 ` Steven Rostedt
2019-03-30 0:26 ` Guo Ren
0 siblings, 1 reply; 9+ messages in thread
From: Steven Rostedt @ 2019-03-29 20:33 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: Will Drewry, Kees Cook, Palmer Dabbelt, linux-kernel,
Andy Lutomirski, Ingo Molnar, Guo Ren, linux-riscv
On Fri, 29 Mar 2019 21:16:19 +0300
"Dmitry V. Levin" <ldv@altlinux.org> wrote:
> This is fine with me. If you are adding the fix for riscv,
> please consider adding the fix for csky, too.
Yes of course. I mentioned both of these fixes in my reply to Linus.
-- Steve
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments()
2019-03-29 20:33 ` Steven Rostedt
@ 2019-03-30 0:26 ` Guo Ren
0 siblings, 0 replies; 9+ messages in thread
From: Guo Ren @ 2019-03-30 0:26 UTC (permalink / raw)
To: Steven Rostedt
Cc: Will Drewry, Kees Cook, Palmer Dabbelt, linux-kernel,
Andy Lutomirski, Ingo Molnar, linux-riscv, Dmitry V. Levin
Thx Dmitry & Steve,
On Fri, Mar 29, 2019 at 04:33:37PM -0400, Steven Rostedt wrote:
> On Fri, 29 Mar 2019 21:16:19 +0300
> "Dmitry V. Levin" <ldv@altlinux.org> wrote:
>
> > This is fine with me. If you are adding the fix for riscv,
> > please consider adding the fix for csky, too.
>
> Yes of course. I mentioned both of these fixes in my reply to Linus.
The BUG is from this commit before upstream:
https://github.com/c-sky/csky-linux/commit/b167422869e6a76b31bda639413efa2ba7e60432#diff-cdc97efef2ab02d6828fa545698f9311
I reference the riscv code without my mind, thx for all.
Best Regards
Guo Ren
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-03-30 0:26 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-29 17:12 [PATCH] riscv: fix syscall_get_arguments() and syscall_set_arguments() Dmitry V. Levin
2019-03-29 17:15 ` Steven Rostedt
2019-03-29 17:52 ` David Abdurachmanov
2019-03-29 17:56 ` Steven Rostedt
2019-03-29 18:11 ` Dmitry V. Levin
2019-03-29 20:32 ` Steven Rostedt
2019-03-29 18:16 ` Dmitry V. Levin
2019-03-29 20:33 ` Steven Rostedt
2019-03-30 0:26 ` Guo Ren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).