2020-09-12 00:59:47

by Kees Cook

[permalink] [raw]
Subject: [PATCH] core/entry: Report syscall correctly for trace and audit

On v5.8 when doing seccomp syscall rewrites (e.g. getpid into getppid
as seen in the seccomp selftests), trace (and audit) correctly see the
rewritten syscall on entry and exit:

seccomp_bpf-1307 [000] .... 22974.874393: sys_enter: NR 110 (...
seccomp_bpf-1307 [000] .N.. 22974.874401: sys_exit: NR 110 = 1304

With mainline we see a mismatched enter and exit (the original syscall
is incorrectly visible on entry):

seccomp_bpf-1030 [000] .... 21.806766: sys_enter: NR 39 (...
seccomp_bpf-1030 [000] .... 21.806767: sys_exit: NR 110 = 1027

When ptrace or seccomp change the syscall, this needs to be visible to
trace and audit at that time as well. Update the syscall earlier so they
see the correct value.

Reported-by: Michael Ellerman <[email protected]>
Fixes: d88d59b64ca3 ("core/entry: Respect syscall number rewrites")
Cc: Thomas Gleixner <[email protected]>
Cc: Kyle Huey <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Ingo Molnar <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
kernel/entry/common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index 18683598edbc..6fdb6105e6d6 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -60,13 +60,15 @@ static long syscall_trace_enter(struct pt_regs *regs, long syscall,
return ret;
}

+ /* Either of the above might have changed the syscall number */
+ syscall = syscall_get_nr(current, regs);
+
if (unlikely(ti_work & _TIF_SYSCALL_TRACEPOINT))
trace_sys_enter(regs, syscall);

syscall_enter_audit(regs, syscall);

- /* The above might have changed the syscall number */
- return ret ? : syscall_get_nr(current, regs);
+ return ret ? : syscall;
}

static __always_inline long
--
2.25.1


2020-09-14 17:25:29

by Kyle Huey

[permalink] [raw]
Subject: Re: [PATCH] core/entry: Report syscall correctly for trace and audit

On Fri, Sep 11, 2020 at 5:58 PM Kees Cook <[email protected]> wrote:
>
> On v5.8 when doing seccomp syscall rewrites (e.g. getpid into getppid
> as seen in the seccomp selftests), trace (and audit) correctly see the
> rewritten syscall on entry and exit:
>
> seccomp_bpf-1307 [000] .... 22974.874393: sys_enter: NR 110 (...
> seccomp_bpf-1307 [000] .N.. 22974.874401: sys_exit: NR 110 = 1304
>
> With mainline we see a mismatched enter and exit (the original syscall
> is incorrectly visible on entry):
>
> seccomp_bpf-1030 [000] .... 21.806766: sys_enter: NR 39 (...
> seccomp_bpf-1030 [000] .... 21.806767: sys_exit: NR 110 = 1027
>
> When ptrace or seccomp change the syscall, this needs to be visible to
> trace and audit at that time as well. Update the syscall earlier so they
> see the correct value.
>
> Reported-by: Michael Ellerman <[email protected]>
> Fixes: d88d59b64ca3 ("core/entry: Respect syscall number rewrites")
> Cc: Thomas Gleixner <[email protected]>
> Cc: Kyle Huey <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> kernel/entry/common.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/entry/common.c b/kernel/entry/common.c
> index 18683598edbc..6fdb6105e6d6 100644
> --- a/kernel/entry/common.c
> +++ b/kernel/entry/common.c
> @@ -60,13 +60,15 @@ static long syscall_trace_enter(struct pt_regs *regs, long syscall,
> return ret;
> }
>
> + /* Either of the above might have changed the syscall number */
> + syscall = syscall_get_nr(current, regs);
> +
> if (unlikely(ti_work & _TIF_SYSCALL_TRACEPOINT))
> trace_sys_enter(regs, syscall);
>
> syscall_enter_audit(regs, syscall);
>
> - /* The above might have changed the syscall number */
> - return ret ? : syscall_get_nr(current, regs);
> + return ret ? : syscall;
> }
>
> static __always_inline long
> --
> 2.25.1
>

lgtm

- Kyle

2020-09-14 20:55:31

by tip-bot2 for Jacob Pan

[permalink] [raw]
Subject: [tip: core/urgent] core/entry: Report syscall correctly for trace and audit

The following commit has been merged into the core/urgent branch of tip:

Commit-ID: b6ec413461034d49f9e586845825adb35ba308f6
Gitweb: https://git.kernel.org/tip/b6ec413461034d49f9e586845825adb35ba308f6
Author: Kees Cook <[email protected]>
AuthorDate: Fri, 11 Sep 2020 17:58:26 -07:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Mon, 14 Sep 2020 22:49:51 +02:00

core/entry: Report syscall correctly for trace and audit

On v5.8 when doing seccomp syscall rewrites (e.g. getpid into getppid
as seen in the seccomp selftests), trace (and audit) correctly see the
rewritten syscall on entry and exit:

seccomp_bpf-1307 [000] .... 22974.874393: sys_enter: NR 110 (...
seccomp_bpf-1307 [000] .N.. 22974.874401: sys_exit: NR 110 = 1304

With mainline we see a mismatched enter and exit (the original syscall
is incorrectly visible on entry):

seccomp_bpf-1030 [000] .... 21.806766: sys_enter: NR 39 (...
seccomp_bpf-1030 [000] .... 21.806767: sys_exit: NR 110 = 1027

When ptrace or seccomp change the syscall, this needs to be visible to
trace and audit at that time as well. Update the syscall earlier so they
see the correct value.

Fixes: d88d59b64ca3 ("core/entry: Respect syscall number rewrites")
Reported-by: Michael Ellerman <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
kernel/entry/common.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/entry/common.c b/kernel/entry/common.c
index 1868359..6fdb610 100644
--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c
@@ -60,13 +60,15 @@ static long syscall_trace_enter(struct pt_regs *regs, long syscall,
return ret;
}

+ /* Either of the above might have changed the syscall number */
+ syscall = syscall_get_nr(current, regs);
+
if (unlikely(ti_work & _TIF_SYSCALL_TRACEPOINT))
trace_sys_enter(regs, syscall);

syscall_enter_audit(regs, syscall);

- /* The above might have changed the syscall number */
- return ret ? : syscall_get_nr(current, regs);
+ return ret ? : syscall;
}

static __always_inline long