2014-01-17 08:13:48

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 0/6] arm64: Add audit support

This patchset adds system call audit support on arm64.
Both 32-bit (AUIDT_ARCH_ARM[EB]) and 64-bit tasks (AUDIT_ARCH_AARCH64[EB])
are supported, but presuming 32-LE on 64-LE or 32-BE on 64-BE.

There are some prerequisites for this patch to work correctly:
* "generic compat system call audit support" patch
* "correct a type mismatch in audit_syscall_exit()" patch
(already accepted and queued in 3.14)
* "Modify a set of system calls in audit class" patch
* userspace audit tool (v2.3.2 + my patch for arm64)

All those were already or will be soon posted separately.
Please review them as well for better understandings.

This code was tested on both 32-bit and 64-bit LE userland
in the following two ways:
1) basic operations with auditctl/autrace
# auditctl -a exit,always -S openat -F path=/etc/inittab
# auditctl -a exit,always -F dir=/tmp -F perm=rw
# auditctl -a task,always
# autrace /bin/ls
by comparing output from autrace with one from strace

2) audit-test-code (+ my workarounds for arm/arm64)
by running "audit-tool", "filter" and "syscalls" test categories.

Changes v1 -> v2:
* Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
Please note that a required header, unistd_32.h, is automatically
generated from unistd32.h.
* Refer to regs->orig_x0 instead of regs->x0 as the first argument of
system call in audit_syscall_entry() [6/6]
* Include "Add regs_return_value() in syscall.h" patch [2/6],
which was not intentionally included in v1 because it could be added
by "kprobes support".


AKASHI Takahiro (6):
audit: Enable arm64 support
arm64: Add regs_return_value() in syscall.h
arm64: Add audit support
arm64: audit: Add 32-bit (compat) syscall support
arm64: audit: Add makefile rule to create unistd_32.h for compat
syscalls
arm64: audit: Add audit hook in ptrace/syscall_trace

arch/arm64/Makefile | 4 ++++
arch/arm64/include/asm/audit.h | 20 ++++++++++++++++++++
arch/arm64/include/asm/ptrace.h | 5 +++++
arch/arm64/include/asm/syscall.h | 22 ++++++++++++++++++++++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/entry.S | 3 +++
arch/arm64/kernel/ptrace.c | 12 ++++++++++++
arch/arm64/kernel/syscalls/Makefile | 20 ++++++++++++++++++++
include/uapi/linux/audit.h | 2 ++
init/Kconfig | 2 +-
10 files changed, 90 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/audit.h
create mode 100644 arch/arm64/kernel/syscalls/Makefile

--
1.7.9.5


2014-01-17 08:14:00

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 1/6] audit: Enable arm64 support

This patch adds AUDIT_ARCH_* identifiers for arm64(AArch64), and
makes CONFIG_AUDITSYSCALL selectable.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
include/uapi/linux/audit.h | 2 ++
init/Kconfig | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 44b05a0..e39635b 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -327,6 +327,8 @@ enum {
/* distinguish syscall tables */
#define __AUDIT_ARCH_64BIT 0x80000000
#define __AUDIT_ARCH_LE 0x40000000
+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
+#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
#define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARMEB (EM_ARM)
diff --git a/init/Kconfig b/init/Kconfig
index 79383d3..3aae602 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -284,7 +284,7 @@ config AUDIT

config AUDITSYSCALL
bool "Enable system-call auditing support"
- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
+ depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
default y if SECURITY_SELINUX
help
Enable low-overhead system-call auditing infrastructure that
--
1.7.9.5

2014-01-17 08:14:11

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 3/6] arm64: Add audit support

On AArch64, audit can be enabled with CONFIG_AUDIT_GENERIC.
Most of audit features are implemented in generic way. This patch
adds a small piece of architecture dependent code.
syscall_get_arch(), which is used in seccomp, should just return
AUDIT_ARCH_*.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/syscall.h | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 70ba9d4..3361fec 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -16,7 +16,9 @@
#ifndef __ASM_SYSCALL_H
#define __ASM_SYSCALL_H

+#include <linux/audit.h>
#include <linux/err.h>
+#include <linux/sched.h>


static inline int syscall_get_nr(struct task_struct *task,
@@ -104,4 +106,14 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->regs[i], args, n * sizeof(args[0]));
}

+static inline int syscall_get_arch(struct task_struct *task,
+ struct pt_regs *regs)
+{
+#ifdef __AARCH64EB__
+ return AUDIT_ARCH_AARCH64EB;
+#else
+ return AUDIT_ARCH_AARCH64;
+#endif
+}
+
#endif /* __ASM_SYSCALL_H */
--
1.7.9.5

2014-01-17 08:14:20

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 4/6] arm64: audit: Add 32-bit (compat) syscall support

Generic audit code also support compat system calls now.
This patch adds a small piece of architecture dependent code.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/audit.h | 20 ++++++++++++++++++++
arch/arm64/include/asm/syscall.h | 10 ++++++++++
2 files changed, 30 insertions(+)
create mode 100644 arch/arm64/include/asm/audit.h

diff --git a/arch/arm64/include/asm/audit.h b/arch/arm64/include/asm/audit.h
new file mode 100644
index 0000000..70eef50
--- /dev/null
+++ b/arch/arm64/include/asm/audit.h
@@ -0,0 +1,20 @@
+/*
+ * arch/arm64/include/asm/audit.h
+ *
+ * Copyright (C) 2013 Linaro Limited
+ * Author: AKASHI Takahiro <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_AUDIT_H
+#define __ASM_AUDIT_H
+
+#include <linux/audit.h>
+
+#define audit_is_compat(arch) \
+ ((arch == AUDIT_ARCH_ARM) || (arch == AUDIT_ARCH_ARMEB))
+
+#endif /* __ASM_AUDIT_H */
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 3361fec..d7660e9 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -19,6 +19,7 @@
#include <linux/audit.h>
#include <linux/err.h>
#include <linux/sched.h>
+#include <asm/compat.h>


static inline int syscall_get_nr(struct task_struct *task,
@@ -109,6 +110,15 @@ static inline void syscall_set_arguments(struct task_struct *task,
static inline int syscall_get_arch(struct task_struct *task,
struct pt_regs *regs)
{
+#ifdef CONFIG_COMPAT
+ if (is_compat_thread(task_thread_info(task)))
+#ifdef __AARCH64EB__
+ return AUDIT_ARCH_ARMEB; /* only BE on BE */
+#else
+ return AUDIT_ARCH_ARM;
+#endif
+#endif
+
#ifdef __AARCH64EB__
return AUDIT_ARCH_AARCH64EB;
#else
--
1.7.9.5

2014-01-17 08:14:26

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 5/6] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls

generic compat sycall audit (lib/compat_audit.c) requires unistd_32.h
for __NR_xyx compat syscall numbers. This is a different file from unistd32.h
on arm64 and so it must be generated from unistd32.h.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/Makefile | 4 ++++
arch/arm64/kernel/syscalls/Makefile | 20 ++++++++++++++++++++
2 files changed, 24 insertions(+)
create mode 100644 arch/arm64/kernel/syscalls/Makefile

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 2fceb71..6d24f92 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -72,6 +72,10 @@ PHONY += vdso_install
vdso_install:
$(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso $@

+# Compat syscall header generation
+archheaders:
+ $(Q)$(MAKE) $(build)=arch/arm64/kernel/syscalls $@
+
# We use MRPROPER_FILES and CLEAN_FILES now
archclean:
$(Q)$(MAKE) $(clean)=$(boot)
diff --git a/arch/arm64/kernel/syscalls/Makefile b/arch/arm64/kernel/syscalls/Makefile
new file mode 100644
index 0000000..7661113
--- /dev/null
+++ b/arch/arm64/kernel/syscalls/Makefile
@@ -0,0 +1,20 @@
+out := $(obj)/../../include/generated/asm
+
+# Create output directory if not already present
+_dummy := $(shell [ -d '$(out)' ] || mkdir -p '$(out)')
+
+syshdr-$(CONFIG_COMPAT) += unistd_32.h
+
+targets += $(syshdr-y)
+
+quiet_cmd_syshdr = SYSHDR $@
+ cmd_syshdr = cat $< | sed -r \
+ -e 's/compat_//' \
+ -e 's/_wrapper//' \
+ -e 's/^__SYSCALL\((.*),[ ]*sys_([^)].*)\).*/\#define __NR_\2 \1/p;d' \
+ | grep -v __NR_ni_syscall > $@
+
+archheaders: $(addprefix $(out)/,$(syshdr-y))
+
+$(out)/unistd_32.h: $(src)/../../include/asm/unistd32.h
+ $(call if_changed,syshdr)
--
1.7.9.5

2014-01-17 08:14:40

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 6/6] arm64: audit: Add audit hook in ptrace/syscall_trace

This patch adds auditing functions on entry to or exit from
every system call invocation.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/entry.S | 3 +++
arch/arm64/kernel/ptrace.c | 12 ++++++++++++
3 files changed, 16 insertions(+)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 720e70b..7468388 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -101,6 +101,7 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NEED_RESCHED 1
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
#define TIF_SYSCALL_TRACE 8
+#define TIF_SYSCALL_AUDIT 9
#define TIF_POLLING_NRFLAG 16
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 4d2c6f3..5bb2c26 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -631,6 +631,9 @@ el0_svc_naked: // compat entry point
get_thread_info tsk
ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
+#ifdef CONFIG_AUDITSYSCALL
+ tbnz x16, #TIF_SYSCALL_AUDIT, __sys_trace // auditing syscalls?
+#endif
adr lr, ret_fast_syscall // return address
cmp scno, sc_nr // check upper syscall limit
b.hs ni_sys
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6777a21..2ca169b 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -19,6 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/audit.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/mm.h>
@@ -38,6 +39,7 @@
#include <asm/compat.h>
#include <asm/debug-monitors.h>
#include <asm/pgtable.h>
+#include <asm/syscall.h>
#include <asm/traps.h>
#include <asm/system_misc.h>

@@ -1064,6 +1066,16 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
{
unsigned long saved_reg;

+#ifdef CONFIG_AUDITSYSCALL
+ if (dir)
+ audit_syscall_exit(regs);
+ else
+ audit_syscall_entry(syscall_get_arch(current, regs),
+ (int)regs->syscallno,
+ regs->orig_x0, regs->regs[1],
+ regs->regs[2], regs->regs[3]);
+#endif /* CONFIG_AUDITSYSCALL */
+
if (!test_thread_flag(TIF_SYSCALL_TRACE))
return regs->syscallno;

--
1.7.9.5

2014-01-17 08:14:08

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 2/6] arm64: Add regs_return_value() in syscall.h

This macro is used mainly for audit to record system call's results, but
may also be used in test_kprobes.c.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/ptrace.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 0e7fa49..5800ec1 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -134,6 +134,11 @@ struct pt_regs {
#define user_stack_pointer(regs) \
((regs)->sp)

+static inline unsigned long regs_return_value(struct pt_regs *regs)
+{
+ return regs->regs[0];
+}
+
/*
* Are the current registers suitable for user mode? (used to maintain
* security in signal handlers)
--
1.7.9.5

2014-01-17 16:47:09

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] arm64: audit: Add 32-bit (compat) syscall support

Hi Akashi,

On Fri, Jan 17, 2014 at 08:13:17AM +0000, AKASHI Takahiro wrote:
> Generic audit code also support compat system calls now.
> This patch adds a small piece of architecture dependent code.

[...]

> static inline int syscall_get_nr(struct task_struct *task,
> @@ -109,6 +110,15 @@ static inline void syscall_set_arguments(struct task_struct *task,
> static inline int syscall_get_arch(struct task_struct *task,
> struct pt_regs *regs)
> {
> +#ifdef CONFIG_COMPAT
> + if (is_compat_thread(task_thread_info(task)))

You can call is_compat_thread even when !CONFIG_COMPAT, so you don't need
that #ifdef.

> +#ifdef __AARCH64EB__
> + return AUDIT_ARCH_ARMEB; /* only BE on BE */

Well, actually, we only support userspace to be the same endianness as the
kernel, so you that comment is slightly misleading. You could probably avoid
these repeated ifdefs by defining things like ARM64_AUDIT_ARCH and
ARM64_COMPAT_AUDIT_ARCH once depending on endianness.

Will

2014-01-17 19:44:46

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 0/6] arm64: Add audit support

On 14/01/17, AKASHI Takahiro wrote:
> This patchset adds system call audit support on arm64.
> Both 32-bit (AUIDT_ARCH_ARM[EB]) and 64-bit tasks (AUDIT_ARCH_AARCH64[EB])
> are supported, but presuming 32-LE on 64-LE or 32-BE on 64-BE.
>
> There are some prerequisites for this patch to work correctly:
> * "generic compat system call audit support" patch
> * "correct a type mismatch in audit_syscall_exit()" patch
> (already accepted and queued in 3.14)
> * "Modify a set of system calls in audit class" patch
> * userspace audit tool (v2.3.2 + my patch for arm64)
>
> All those were already or will be soon posted separately.
> Please review them as well for better understandings.
>
> This code was tested on both 32-bit and 64-bit LE userland
> in the following two ways:
> 1) basic operations with auditctl/autrace
> # auditctl -a exit,always -S openat -F path=/etc/inittab
> # auditctl -a exit,always -F dir=/tmp -F perm=rw
> # auditctl -a task,always
> # autrace /bin/ls
> by comparing output from autrace with one from strace
>
> 2) audit-test-code (+ my workarounds for arm/arm64)
> by running "audit-tool", "filter" and "syscalls" test categories.
>
> Changes v1 -> v2:
> * Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
> Please note that a required header, unistd_32.h, is automatically
> generated from unistd32.h.
> * Refer to regs->orig_x0 instead of regs->x0 as the first argument of
> system call in audit_syscall_entry() [6/6]
> * Include "Add regs_return_value() in syscall.h" patch [2/6],
> which was not intentionally included in v1 because it could be added
> by "kprobes support".
>
>
> AKASHI Takahiro (6):
> audit: Enable arm64 support
> arm64: Add regs_return_value() in syscall.h
> arm64: Add audit support
> arm64: audit: Add 32-bit (compat) syscall support
> arm64: audit: Add makefile rule to create unistd_32.h for compat
> syscalls
> arm64: audit: Add audit hook in ptrace/syscall_trace
>
> arch/arm64/Makefile | 4 ++++
> arch/arm64/include/asm/audit.h | 20 ++++++++++++++++++++
> arch/arm64/include/asm/ptrace.h | 5 +++++
> arch/arm64/include/asm/syscall.h | 22 ++++++++++++++++++++++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/kernel/entry.S | 3 +++
> arch/arm64/kernel/ptrace.c | 12 ++++++++++++
> arch/arm64/kernel/syscalls/Makefile | 20 ++++++++++++++++++++
> include/uapi/linux/audit.h | 2 ++
> init/Kconfig | 2 +-
> 10 files changed, 90 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/include/asm/audit.h
> create mode 100644 arch/arm64/kernel/syscalls/Makefile
>
> --
> 1.7.9.5

Set:
Acked-by: Richard Guy Briggs <[email protected]>

>

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-01-20 05:23:35

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v2 4/6] arm64: audit: Add 32-bit (compat) syscall support

On 01/18/2014 01:46 AM, Will Deacon wrote:
> Hi Akashi,
>
> On Fri, Jan 17, 2014 at 08:13:17AM +0000, AKASHI Takahiro wrote:
>> Generic audit code also support compat system calls now.
>> This patch adds a small piece of architecture dependent code.
>
> [...]
>
>> static inline int syscall_get_nr(struct task_struct *task,
>> @@ -109,6 +110,15 @@ static inline void syscall_set_arguments(struct task_struct *task,
>> static inline int syscall_get_arch(struct task_struct *task,
>> struct pt_regs *regs)
>> {
>> +#ifdef CONFIG_COMPAT
>> + if (is_compat_thread(task_thread_info(task)))
>
> You can call is_compat_thread even when !CONFIG_COMPAT, so you don't need
> that #ifdef.

Right. I will remove it.

>> +#ifdef __AARCH64EB__
>> + return AUDIT_ARCH_ARMEB; /* only BE on BE */
>
> Well, actually, we only support userspace to be the same endianness as the
> kernel, so you that comment is slightly misleading. You could probably avoid
> these repeated ifdefs by defining things like ARM64_AUDIT_ARCH and
> ARM64_COMPAT_AUDIT_ARCH once depending on endianness.

As in the discussions about "audit(userspace)", if we don't have to care
about endianness, I will remove this #ifdef instead.

Thanks,
-Takahiro AKASHI

> Will
>

2014-01-23 14:18:45

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -327,6 +327,8 @@ enum {
> /* distinguish syscall tables */
> #define __AUDIT_ARCH_64BIT 0x80000000
> #define __AUDIT_ARCH_LE 0x40000000
> +#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> +#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_ARMEB (EM_ARM)
> diff --git a/init/Kconfig b/init/Kconfig
> index 79383d3..3aae602 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -284,7 +284,7 @@ config AUDIT
>
> config AUDITSYSCALL
> bool "Enable system-call auditing support"
> - depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> + depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)

The usual comment for such changes: could you please clean this up and
just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?

--
Catalin

2014-01-23 14:54:46

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls

On Fri, Jan 17, 2014 at 08:13:18AM +0000, AKASHI Takahiro wrote:
> generic compat sycall audit (lib/compat_audit.c) requires unistd_32.h
> for __NR_xyx compat syscall numbers. This is a different file from unistd32.h
> on arm64 and so it must be generated from unistd32.h.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/arm64/Makefile | 4 ++++
> arch/arm64/kernel/syscalls/Makefile | 20 ++++++++++++++++++++
> 2 files changed, 24 insertions(+)
> create mode 100644 arch/arm64/kernel/syscalls/Makefile
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 2fceb71..6d24f92 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -72,6 +72,10 @@ PHONY += vdso_install
> vdso_install:
> $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso $@
>
> +# Compat syscall header generation
> +archheaders:
> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/syscalls $@

See my other post to the lib/compat_audit.c file. I think that's too
complex for what you need.

--
Catalin

2014-01-23 14:56:40

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 6/6] arm64: audit: Add audit hook in ptrace/syscall_trace

On Fri, Jan 17, 2014 at 08:13:19AM +0000, AKASHI Takahiro wrote:
> @@ -1064,6 +1066,16 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> {
> unsigned long saved_reg;
>
> +#ifdef CONFIG_AUDITSYSCALL
> + if (dir)
> + audit_syscall_exit(regs);
> + else
> + audit_syscall_entry(syscall_get_arch(current, regs),
> + (int)regs->syscallno,
> + regs->orig_x0, regs->regs[1],
> + regs->regs[2], regs->regs[3]);
> +#endif /* CONFIG_AUDITSYSCALL */

It should work without the #ifdef as audit_syscall_exit/entry are dummy
static inline functions when !CONFIG_AUDITSYSCALL.

--
Catalin

2014-01-27 05:12:44

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

[To audit maintainers]

On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
>> --- a/include/uapi/linux/audit.h
>> +++ b/include/uapi/linux/audit.h
>> @@ -327,6 +327,8 @@ enum {
>> /* distinguish syscall tables */
>> #define __AUDIT_ARCH_64BIT 0x80000000
>> #define __AUDIT_ARCH_LE 0x40000000
>> +#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>> +#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
>> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
>> #define AUDIT_ARCH_ARMEB (EM_ARM)
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 79383d3..3aae602 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -284,7 +284,7 @@ config AUDIT
>>
>> config AUDITSYSCALL
>> bool "Enable system-call auditing support"
>> - depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
>> + depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
>
> The usual comment for such changes: could you please clean this up and
> just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?

Do you agree to this change?

If so, I can create a patch, but have some concerns:
1) I can't verify it on other architectures than (arm &) arm64.
2) Some architectures (microblaze, mips, openrisc) are not listed here, but
their ptrace.c have a call to audit_syscall_entry/exit().
(audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
So I'm afraid that the change might break someone's assumption.

Thanks,
-Takahiro AKASHI

2014-01-27 06:13:43

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v2 5/6] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls

Catalin,

On 01/23/2014 11:53 PM, Catalin Marinas wrote:
> On Fri, Jan 17, 2014 at 08:13:18AM +0000, AKASHI Takahiro wrote:
>> generic compat sycall audit (lib/compat_audit.c) requires unistd_32.h
>> for __NR_xyx compat syscall numbers. This is a different file from unistd32.h
>> on arm64 and so it must be generated from unistd32.h.
>>
>> Signed-off-by: AKASHI Takahiro <[email protected]>
>> ---
>> arch/arm64/Makefile | 4 ++++
>> arch/arm64/kernel/syscalls/Makefile | 20 ++++++++++++++++++++
>> 2 files changed, 24 insertions(+)
>> create mode 100644 arch/arm64/kernel/syscalls/Makefile
>>
>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>> index 2fceb71..6d24f92 100644
>> --- a/arch/arm64/Makefile
>> +++ b/arch/arm64/Makefile
>> @@ -72,6 +72,10 @@ PHONY += vdso_install
>> vdso_install:
>> $(Q)$(MAKE) $(build)=arch/arm64/kernel/vdso $@
>>
>> +# Compat syscall header generation
>> +archheaders:
>> + $(Q)$(MAKE) $(build)=arch/arm64/kernel/syscalls $@
>
> See my other post to the lib/compat_audit.c file. I think that's too
> complex for what you need.

Generation script is getting more complexed than I assumed at first
because some of system call names are a bit inconsistent with native 32-bit
system calls, for example, fchown16 vs. fchown, fchown vs. fchown32.

Now my tentative sed script looks like:
+quiet_cmd_syshdr = SYSHDR $@
+ cmd_syshdr = cat $< | sed -r \
+ -e 's/compat_//' \
+ -e 's/_wrapper//' \
+ -e 's/(sys_[fl]?chown)(\))/\132\)/' \
+ -e 's/(sys_[gs]et)(|e|fs|re|res)(uid\))/\1\2uid32\)/' \
+ -e 's/(sys_[gs]et)(|e|fs|re|res)(gid\))/\1\2gid32\)/' \
+ -e 's/(sys_[gs]etgroups)(\))/\132\)/' \
+ -e 's/(sys_new)(.*)/sys_\2/' \
+ -e 's/sys_mmap_pgoff/sys_mmap2/' \
+ -e 's/(sys_[_a-z]*)16(.*)/\1\2/' \
+ -e 's/^__SYSCALL\((.*),[ ]*sys_([^)].*)\).*/\#define __NR_\2 \1/p;d' \
+ | grep -v __NR_ni_syscall > $@

So, yeah, I agree with you now.

-Takahiro AKASHI

2014-01-27 14:50:00

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

On Mon, Jan 27, 2014 at 05:12:33AM +0000, AKASHI Takahiro wrote:
> [To audit maintainers]
>
> On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> > On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> >> --- a/include/uapi/linux/audit.h
> >> +++ b/include/uapi/linux/audit.h
> >> @@ -327,6 +327,8 @@ enum {
> >> /* distinguish syscall tables */
> >> #define __AUDIT_ARCH_64BIT 0x80000000
> >> #define __AUDIT_ARCH_LE 0x40000000
> >> +#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >> +#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> >> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> >> #define AUDIT_ARCH_ARMEB (EM_ARM)
> >> diff --git a/init/Kconfig b/init/Kconfig
> >> index 79383d3..3aae602 100644
> >> --- a/init/Kconfig
> >> +++ b/init/Kconfig
> >> @@ -284,7 +284,7 @@ config AUDIT
> >>
> >> config AUDITSYSCALL
> >> bool "Enable system-call auditing support"
> >> - depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> >> + depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> >
> > The usual comment for such changes: could you please clean this up and
> > just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
>
> Do you agree to this change?
>
> If so, I can create a patch, but have some concerns:
> 1) I can't verify it on other architectures than (arm &) arm64.

You could try to build. It's really a trivial change, could get away
with code inspection (and some automatic building when it gets to
linux-next).

In init/Kconfig:

config HAVE_ARCH_AUDITSYSCALL
bool

and:

- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
+ depends on HAVE_ARCH_AUDITSYSCALL

In the corresponding arch/*/Kconfig:

select HAVE_ARCH_AUDITSYSCALL

> 2) Some architectures (microblaze, mips, openrisc) are not listed here, but

For those, you don't need to select HAVE_ARCH_AUDITSYSCALL.

> their ptrace.c have a call to audit_syscall_entry/exit().
> (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)


They are not NULL but empty inline functions, so they don't have any
effect.

> So I'm afraid that the change might break someone's assumption.

I'm pretty sure it won't ;).

--
Catalin

2014-01-29 20:22:14

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

On 14/01/27, AKASHI Takahiro wrote:
> [To audit maintainers]
>
> On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> >On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> >>--- a/include/uapi/linux/audit.h
> >>+++ b/include/uapi/linux/audit.h
> >>@@ -327,6 +327,8 @@ enum {
> >> /* distinguish syscall tables */
> >> #define __AUDIT_ARCH_64BIT 0x80000000
> >> #define __AUDIT_ARCH_LE 0x40000000
> >>+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >>+#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> >> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> >> #define AUDIT_ARCH_ARMEB (EM_ARM)
> >>diff --git a/init/Kconfig b/init/Kconfig
> >>index 79383d3..3aae602 100644
> >>--- a/init/Kconfig
> >>+++ b/init/Kconfig
> >>@@ -284,7 +284,7 @@ config AUDIT
> >>
> >> config AUDITSYSCALL
> >> bool "Enable system-call auditing support"
> >>- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> >>+ depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> >
> >The usual comment for such changes: could you please clean this up and
> >just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
>
> Do you agree to this change?
>
> If so, I can create a patch, but have some concerns:
> 1) I can't verify it on other architectures than (arm &) arm64.
> 2) Some architectures (microblaze, mips, openrisc) are not listed here, but
> their ptrace.c have a call to audit_syscall_entry/exit().
> (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)

I can try: ppc s390 x86_64 ppc64 i686 s390x

> So I'm afraid that the change might break someone's assumption.
>
> Thanks,
> -Takahiro AKASHI

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-01-29 22:37:33

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

On 14/01/29, Richard Guy Briggs wrote:
> On 14/01/27, AKASHI Takahiro wrote:
> > [To audit maintainers]
> >
> > On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> > >On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> > >>--- a/include/uapi/linux/audit.h
> > >>+++ b/include/uapi/linux/audit.h
> > >>@@ -327,6 +327,8 @@ enum {
> > >> /* distinguish syscall tables */
> > >> #define __AUDIT_ARCH_64BIT 0x80000000
> > >> #define __AUDIT_ARCH_LE 0x40000000
> > >>+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > >>+#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> > >> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > >> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> > >> #define AUDIT_ARCH_ARMEB (EM_ARM)
> > >>diff --git a/init/Kconfig b/init/Kconfig
> > >>index 79383d3..3aae602 100644
> > >>--- a/init/Kconfig
> > >>+++ b/init/Kconfig
> > >>@@ -284,7 +284,7 @@ config AUDIT
> > >>
> > >> config AUDITSYSCALL
> > >> bool "Enable system-call auditing support"
> > >>- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> > >>+ depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> > >
> > >The usual comment for such changes: could you please clean this up and
> > >just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
> >
> > Do you agree to this change?
> >
> > If so, I can create a patch, but have some concerns:
> > 1) I can't verify it on other architectures than (arm &) arm64.
> > 2) Some architectures (microblaze, mips, openrisc) are not listed here, but
> > their ptrace.c have a call to audit_syscall_entry/exit().
> > (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
>
> I can try: ppc s390 x86_64 ppc64 i686 s390x

These arches above all pass compile and basic tests with the following patches applied:

audit: correct a type mismatch in audit_syscall_exit() pending (already upstream)

audit: Modify a set of system calls in audit class definitions (already upstream)

[PATCH v3] audit: Add generic compat syscall support

[PATCH v2] audit: Enable arm64 support
[PATCH v2] arm64: Add regs_return_value() in syscall.h
[PATCH v2] arm64: Add audit support
[PATCH v2] arm64: audit: Add 32-bit (compat) syscall support
[PATCH v2] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls
[PATCH v2] arm64: audit: Add audit hook in ptrace/syscall_trace

> > So I'm afraid that the change might break someone's assumption.
> >
> > Thanks,
> > -Takahiro AKASHI
>
> - RGB

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-02-03 05:59:53

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

Richard,

On 01/30/2014 07:36 AM, Richard Guy Briggs wrote:
> On 14/01/29, Richard Guy Briggs wrote:
>> On 14/01/27, AKASHI Takahiro wrote:
>>> [To audit maintainers]
>>>
>>> On 01/23/2014 11:18 PM, Catalin Marinas wrote:
>>>> On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
>>>>> --- a/include/uapi/linux/audit.h
>>>>> +++ b/include/uapi/linux/audit.h
>>>>> @@ -327,6 +327,8 @@ enum {
>>>>> /* distinguish syscall tables */
>>>>> #define __AUDIT_ARCH_64BIT 0x80000000
>>>>> #define __AUDIT_ARCH_LE 0x40000000
>>>>> +#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>>>>> +#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
>>>>> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>>>>> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
>>>>> #define AUDIT_ARCH_ARMEB (EM_ARM)
>>>>> diff --git a/init/Kconfig b/init/Kconfig
>>>>> index 79383d3..3aae602 100644
>>>>> --- a/init/Kconfig
>>>>> +++ b/init/Kconfig
>>>>> @@ -284,7 +284,7 @@ config AUDIT
>>>>>
>>>>> config AUDITSYSCALL
>>>>> bool "Enable system-call auditing support"
>>>>> - depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
>>>>> + depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
>>>>
>>>> The usual comment for such changes: could you please clean this up and
>>>> just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
>>>
>>> Do you agree to this change?
>>>
>>> If so, I can create a patch, but have some concerns:
>>> 1) I can't verify it on other architectures than (arm &) arm64.
>>> 2) Some architectures (microblaze, mips, openrisc) are not listed here, but
>>> their ptrace.c have a call to audit_syscall_entry/exit().
>>> (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
>>
>> I can try: ppc s390 x86_64 ppc64 i686 s390x
>
> These arches above all pass compile and basic tests with the following patches applied:
>
> audit: correct a type mismatch in audit_syscall_exit() pending (already upstream)
>
> audit: Modify a set of system calls in audit class definitions (already upstream)
>
> [PATCH v3] audit: Add generic compat syscall support
>
> [PATCH v2] audit: Enable arm64 support
> [PATCH v2] arm64: Add regs_return_value() in syscall.h
> [PATCH v2] arm64: Add audit support
> [PATCH v2] arm64: audit: Add 32-bit (compat) syscall support
> [PATCH v2] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls
> [PATCH v2] arm64: audit: Add audit hook in ptrace/syscall_trace

I think that you missed Catalin's suggestion.
Please use the patch I will post after this message and try it again, please?

Thanks,
-Takahiro AKASHI



>>> So I'm afraid that the change might break someone's assumption.
>>>
>>> Thanks,
>>> -Takahiro AKASHI
>>
>> - RGB
>
> - RGB
>
> --
> Richard Guy Briggs <[email protected]>
> Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
>

2014-02-03 06:01:11

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH] audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

Currently AUDITSYSCALL has a long list of architecture depencency:
depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML ||
SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
The purpose of this patch is to replace it with HAVE_ARCH_AUDITSYSCALL
for simplicity.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/parisc/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/um/Kconfig.common | 1 +
arch/x86/Kconfig | 1 +
init/Kconfig | 5 ++++-
10 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c1f1a7e..cf69f89 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -23,6 +23,7 @@ config ARM
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select HARDIRQS_SW_RESEND
+ select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 4e4119b..9143d91 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -43,6 +43,7 @@ config IA64
select HAVE_MOD_ARCH_SPECIFIC
select MODULES_USE_ELF_RELA
select ARCH_USE_CMPXCHG_LOCKREF
+ select HAVE_ARCH_AUDITSYSCALL
default y
help
The Itanium Processor Family is Intel's 64-bit successor to
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index b5f1858..0821e83 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -28,6 +28,7 @@ config PARISC
select CLONE_BACKWARDS
select TTY # Needed for pdc_cons.c
select HAVE_DEBUG_STACKOVERFLOW
+ select HAVE_ARCH_AUDITSYSCALL

help
The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index b44b52c..96627d6 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -139,6 +139,7 @@ config PPC
select OLD_SIGACTION if PPC32
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_IRQ_EXIT_ON_IRQ_STACK
+ select HAVE_ARCH_AUDITSYSCALL

config GENERIC_CSUM
def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 1e1a03d..b3b9853 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -103,6 +103,7 @@ config S390
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
select HAVE_ALIGNED_STRUCT_PAGE if SLUB
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL if !MARCH_G5
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 9b0979f..675fb7c 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -42,6 +42,7 @@ config SUPERH
select MODULES_USE_ELF_RELA
select OLD_SIGSUSPEND
select OLD_SIGACTION
+ select HAVE_ARCH_AUDITSYSCALL
help
The SuperH is a RISC processor targeted for use in embedded systems
and consumer electronics; it was also used in the Sega Dreamcast
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index d4f7a6a..7f7ad7e 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -76,6 +76,7 @@ config SPARC64
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select HAVE_C_RECORDMCOUNT
select NO_BOOTMEM
+ select HAVE_ARCH_AUDITSYSCALL

config ARCH_DEFCONFIG
string
diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
index 21ca44c..6915d28 100644
--- a/arch/um/Kconfig.common
+++ b/arch/um/Kconfig.common
@@ -1,6 +1,7 @@
config UML
bool
default y
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_UID16
select GENERIC_IRQ_SHOW
select GENERIC_CPU_DEVICES
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e903c71..6ef682f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -124,6 +124,7 @@ config X86
select RTC_LIB
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
+ select HAVE_ARCH_AUDITSYSCALL

config INSTRUCTION_DECODER
def_bool y
diff --git a/init/Kconfig b/init/Kconfig
index 79383d3..9fe22d2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -282,9 +282,12 @@ config AUDIT
logging of avc messages output). Does not do system-call
auditing without CONFIG_AUDITSYSCALL.

+config HAVE_ARCH_AUDITSYSCALL
+ bool
+
config AUDITSYSCALL
bool "Enable system-call auditing support"
- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
+ depends on AUDIT && HAVE_ARCH_AUDITSYSCALL
default y if SECURITY_SELINUX
help
Enable low-overhead system-call auditing infrastructure that
--
1.7.9.5

2014-02-03 06:56:53

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 0/3] arm64: Add audit support

This patchset adds system call audit support on arm64.
Both 32-bit (AUDIT_ARCH_ARM) and 64-bit tasks (AUDIT_ARCH_AARCH64)
are supported. Since arm64 has the exact same set of system calls
on LE and BE, we don't care about endianness (or more specifically
__AUDIT_ARCH_64BIT bit in AUDIT_ARCH_*).

There are some prerequisites for this patch to work correctly:
* "generic compat system call audit support" patch
* "correct a type mismatch in audit_syscall_exit()" patch
(already accepted and queued in 3.14)
* "Modify a set of system calls in audit class" patch
(already accepted and queued in 3.14)
* "__NR_* definitions for compat syscalls" patch from Catalin
* userspace audit tool (v2.3.2 + my patch for arm64)

Please review them as well for better understandings.

This code was tested on both 32-bit and 64-bit LE userland
in the following two ways:
1) basic operations with auditctl/autrace
# auditctl -a exit,always -S openat -F path=/etc/inittab
# auditctl -a exit,always -F dir=/tmp -F perm=rw
# auditctl -a task,always
# autrace /bin/ls
by comparing output from autrace with one from strace

2) audit-test-code (+ my workarounds for arm/arm64)
by running "audit-tool", "filter" and "syscalls" test categories.

Changes v1 -> v2:
* Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
Please note that a required header, unistd_32.h, is automatically
generated from unistd32.h.
* Refer to regs->orig_x0 instead of regs->x0 as the first argument of
system call in audit_syscall_entry() [6/6]
* Include "Add regs_return_value() in syscall.h" patch [2/6],
which was not intentionally included in v1 because it could be added
by "kprobes support".

Changes v2 -> v3:
* Remove asm/audit.h.
See "generic compat syscall audit support" patch v4
* Remove endianness dependency, ie. AUDIT_ARCH_ARMEB/AARCH64EB.
* Remove kernel/syscalls/Makefile which was used to create unistd32.h.
See Catalin's "Add __NR_* definitions for compat syscalls" patch

AKASHI Takahiro (3):
arm64: Add regs_return_value() in syscall.h
arm64: Add audit support
arm64: audit: Add audit hook in ptrace/syscall_trace

arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/ptrace.h | 5 +++++
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/entry.S | 3 +++
arch/arm64/kernel/ptrace.c | 10 ++++++++++
include/uapi/linux/audit.h | 1 +
7 files changed, 36 insertions(+)

--
1.7.9.5

2014-02-03 06:57:01

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 1/3] arm64: Add regs_return_value() in syscall.h

This macro, regs_return_value, is used mainly for audit to record system
call's results, but may also be used in test_kprobes.c.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/ptrace.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 0e7fa49..5800ec1 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -134,6 +134,11 @@ struct pt_regs {
#define user_stack_pointer(regs) \
((regs)->sp)

+static inline unsigned long regs_return_value(struct pt_regs *regs)
+{
+ return regs->regs[0];
+}
+
/*
* Are the current registers suitable for user mode? (used to maintain
* security in signal handlers)
--
1.7.9.5

2014-02-03 06:57:15

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 2/3] arm64: Add audit support

On AArch64, audit is supported through generic lib/audit.c and
compat_audit.c, and so this patch adds arch specific definitions required.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
include/uapi/linux/audit.h | 1 +
3 files changed, 17 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 6d4dd22..3c21405 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -19,6 +19,7 @@ config ARM64
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
select HARDIRQS_SW_RESEND
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_TRACEHOOK
select HAVE_DEBUG_BUGVERBOSE
select HAVE_DEBUG_KMEMLEAK
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 70ba9d4..6900183 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -16,7 +16,9 @@
#ifndef __ASM_SYSCALL_H
#define __ASM_SYSCALL_H

+#include <linux/audit.h>
#include <linux/err.h>
+#include <asm/compat.h>


static inline int syscall_get_nr(struct task_struct *task,
@@ -104,4 +106,17 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->regs[i], args, n * sizeof(args[0]));
}

+/*
+ * We don't care about endianness (__AUDIT_ARCH_LE bit) here because
+ * AArch64 has the same system calls both on little- and big- endian.
+ */
+static inline int syscall_get_arch(struct task_struct *task,
+ struct pt_regs *regs)
+{
+ if (is_compat_thread(task_thread_info(task)))
+ return AUDIT_ARCH_ARM;
+
+ return AUDIT_ARCH_AARCH64;
+}
+
#endif /* __ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 0a73cf3..cf27cae 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -327,6 +327,7 @@ enum {
/* distinguish syscall tables */
#define __AUDIT_ARCH_64BIT 0x80000000
#define __AUDIT_ARCH_LE 0x40000000
+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARMEB (EM_ARM)
--
1.7.9.5

2014-02-03 06:57:24

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 3/3] arm64: audit: Add audit hook in ptrace/syscall_trace

This patch adds auditing functions on entry to or exit from
every system call invocation.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/entry.S | 3 +++
arch/arm64/kernel/ptrace.c | 10 ++++++++++
3 files changed, 14 insertions(+)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 720e70b..7468388 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -101,6 +101,7 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NEED_RESCHED 1
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
#define TIF_SYSCALL_TRACE 8
+#define TIF_SYSCALL_AUDIT 9
#define TIF_POLLING_NRFLAG 16
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 827cbad..83c4b29 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -630,6 +630,9 @@ el0_svc_naked: // compat entry point
get_thread_info tsk
ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
+#ifdef CONFIG_AUDITSYSCALL
+ tbnz x16, #TIF_SYSCALL_AUDIT, __sys_trace // auditing syscalls?
+#endif
adr lr, ret_fast_syscall // return address
cmp scno, sc_nr // check upper syscall limit
b.hs ni_sys
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6777a21..75a3f23 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -19,6 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/audit.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/mm.h>
@@ -38,6 +39,7 @@
#include <asm/compat.h>
#include <asm/debug-monitors.h>
#include <asm/pgtable.h>
+#include <asm/syscall.h>
#include <asm/traps.h>
#include <asm/system_misc.h>

@@ -1064,6 +1066,14 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
{
unsigned long saved_reg;

+ if (dir)
+ audit_syscall_exit(regs);
+ else
+ audit_syscall_entry(syscall_get_arch(current, regs),
+ (int)regs->syscallno,
+ regs->orig_x0, regs->regs[1],
+ regs->regs[2], regs->regs[3]);
+
if (!test_thread_flag(TIF_SYSCALL_TRACE))
return regs->syscallno;

--
1.7.9.5

2014-02-03 16:07:45

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

On 14/02/03, AKASHI Takahiro wrote:
> Richard,

Takahiro,

> On 01/30/2014 07:36 AM, Richard Guy Briggs wrote:
> >On 14/01/29, Richard Guy Briggs wrote:
> >>On 14/01/27, AKASHI Takahiro wrote:
> >>>[To audit maintainers]
> >>>
> >>>On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> >>>>On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> >>>>>--- a/include/uapi/linux/audit.h
> >>>>>+++ b/include/uapi/linux/audit.h
> >>>>>@@ -327,6 +327,8 @@ enum {
> >>>>> /* distinguish syscall tables */
> >>>>> #define __AUDIT_ARCH_64BIT 0x80000000
> >>>>> #define __AUDIT_ARCH_LE 0x40000000
> >>>>>+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >>>>>+#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> >>>>> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> >>>>> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> >>>>> #define AUDIT_ARCH_ARMEB (EM_ARM)
> >>>>>diff --git a/init/Kconfig b/init/Kconfig
> >>>>>index 79383d3..3aae602 100644
> >>>>>--- a/init/Kconfig
> >>>>>+++ b/init/Kconfig
> >>>>>@@ -284,7 +284,7 @@ config AUDIT
> >>>>>
> >>>>> config AUDITSYSCALL
> >>>>> bool "Enable system-call auditing support"
> >>>>>- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> >>>>>+ depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> >>>>
> >>>>The usual comment for such changes: could you please clean this up and
> >>>>just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
> >>>
> >>>Do you agree to this change?
> >>>
> >>>If so, I can create a patch, but have some concerns:
> >>>1) I can't verify it on other architectures than (arm &) arm64.
> >>>2) Some architectures (microblaze, mips, openrisc) are not listed here, but
> >>> their ptrace.c have a call to audit_syscall_entry/exit().
> >>> (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
> >>
> >>I can try: ppc s390 x86_64 ppc64 i686 s390x
> >
> >These arches above all pass compile and basic tests with the following patches applied:
> >
> > audit: correct a type mismatch in audit_syscall_exit() pending (already upstream)
> >
> > audit: Modify a set of system calls in audit class definitions (already upstream)
> >
> > [PATCH v3] audit: Add generic compat syscall support
> >
> > [PATCH v2] audit: Enable arm64 support
> > [PATCH v2] arm64: Add regs_return_value() in syscall.h
> > [PATCH v2] arm64: Add audit support
> > [PATCH v2] arm64: audit: Add 32-bit (compat) syscall support
> > [PATCH v2] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls
> > [PATCH v2] arm64: audit: Add audit hook in ptrace/syscall_trace
>
> I think that you missed Catalin's suggestion.

I didn't miss his suggestions. I think they are a good way to go, but I
wanted to make a test at referrable point in time to validate the work
to that point and to avoid introducing errors by mis-interpreting ideas
that were not yet fully-formed patches.

> Please use the patch I will post after this message and try it again, please?

I was certainly intending to do so.

> Thanks,
> -Takahiro AKASHI
>
> >>>So I'm afraid that the change might break someone's assumption.
> >>>
> >>>Thanks,
> >>>-Takahiro AKASHI
> >>
> >>- RGB
> >
> >- RGB
> >
> >--
> >Richard Guy Briggs <[email protected]>
> >Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
> >Remote, Ottawa, Canada
> >Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
> >

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-02-04 16:25:50

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 1/6] audit: Enable arm64 support

On 14/02/03, Richard Guy Briggs wrote:
> On 14/02/03, AKASHI Takahiro wrote:
> > Richard,
>
> Takahiro,

Takahiro,

> > On 01/30/2014 07:36 AM, Richard Guy Briggs wrote:
> > >On 14/01/29, Richard Guy Briggs wrote:
> > >>On 14/01/27, AKASHI Takahiro wrote:
> > >>>[To audit maintainers]
> > >>>
> > >>>On 01/23/2014 11:18 PM, Catalin Marinas wrote:
> > >>>>On Fri, Jan 17, 2014 at 08:13:14AM +0000, AKASHI Takahiro wrote:
> > >>>>>--- a/include/uapi/linux/audit.h
> > >>>>>+++ b/include/uapi/linux/audit.h
> > >>>>>@@ -327,6 +327,8 @@ enum {
> > >>>>> /* distinguish syscall tables */
> > >>>>> #define __AUDIT_ARCH_64BIT 0x80000000
> > >>>>> #define __AUDIT_ARCH_LE 0x40000000
> > >>>>>+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > >>>>>+#define AUDIT_ARCH_AARCH64EB (EM_AARCH64|__AUDIT_ARCH_64BIT)
> > >>>>> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > >>>>> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> > >>>>> #define AUDIT_ARCH_ARMEB (EM_ARM)
> > >>>>>diff --git a/init/Kconfig b/init/Kconfig
> > >>>>>index 79383d3..3aae602 100644
> > >>>>>--- a/init/Kconfig
> > >>>>>+++ b/init/Kconfig
> > >>>>>@@ -284,7 +284,7 @@ config AUDIT
> > >>>>>
> > >>>>> config AUDITSYSCALL
> > >>>>> bool "Enable system-call auditing support"
> > >>>>>- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT))
> > >>>>>+ depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ARM64)
> > >>>>
> > >>>>The usual comment for such changes: could you please clean this up and
> > >>>>just use something like "depends on HAVE_ARCH_AUDITSYSCALL"?
> > >>>
> > >>>Do you agree to this change?
> > >>>
> > >>>If so, I can create a patch, but have some concerns:
> > >>>1) I can't verify it on other architectures than (arm &) arm64.
> > >>>2) Some architectures (microblaze, mips, openrisc) are not listed here, but
> > >>> their ptrace.c have a call to audit_syscall_entry/exit().
> > >>> (audit_syscall_entry/exit are null if !AUDITSYSCALL, though)
> > >>
> > >>I can try: ppc s390 x86_64 ppc64 i686 s390x
> > >
> > >These arches above all pass compile and basic tests with the following patches applied:
> > >
> > > audit: correct a type mismatch in audit_syscall_exit() pending (already upstream)
> > >
> > > audit: Modify a set of system calls in audit class definitions (already upstream)
> > >
> > > [PATCH v3] audit: Add generic compat syscall support
> > >
> > > [PATCH v2] audit: Enable arm64 support
> > > [PATCH v2] arm64: Add regs_return_value() in syscall.h
> > > [PATCH v2] arm64: Add audit support
> > > [PATCH v2] arm64: audit: Add 32-bit (compat) syscall support
> > > [PATCH v2] arm64: audit: Add makefile rule to create unistd_32.h for compat syscalls
> > > [PATCH v2] arm64: audit: Add audit hook in ptrace/syscall_trace
> >
> > I think that you missed Catalin's suggestion.
>
> I didn't miss his suggestions. I think they are a good way to go, but I
> wanted to make a test at referrable point in time to validate the work
> to that point and to avoid introducing errors by mis-interpreting ideas
> that were not yet fully-formed patches.
>
> > Please use the patch I will post after this message and try it again, please?
>
> I was certainly intending to do so.

I have tested the new sets from Catalin and you and everything passes ok.

> > Thanks,
> > -Takahiro AKASHI
> >
> > >>>So I'm afraid that the change might break someone's assumption.
> > >>>
> > >>>Thanks,
> > >>>-Takahiro AKASHI
> > >>
> > >>- RGB
> > >
> > >- RGB
>
> - RGB

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-02-04 17:30:47

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 1/3] arm64: Add regs_return_value() in syscall.h

On Mon, Feb 03, 2014 at 06:56:28AM +0000, AKASHI Takahiro wrote:
> This macro, regs_return_value, is used mainly for audit to record system
> call's results, but may also be used in test_kprobes.c.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>

Acked-by: Will Deacon <[email protected]>

Will

2014-02-04 17:30:55

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 2/3] arm64: Add audit support

On Mon, Feb 03, 2014 at 06:56:29AM +0000, AKASHI Takahiro wrote:
> On AArch64, audit is supported through generic lib/audit.c and
> compat_audit.c, and so this patch adds arch specific definitions required.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
> include/uapi/linux/audit.h | 1 +
> 3 files changed, 17 insertions(+)

Acked-by: Will Deacon <[email protected]>

Will

2014-02-04 17:32:17

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: audit: Add audit hook in ptrace/syscall_trace

On Mon, Feb 03, 2014 at 06:56:30AM +0000, AKASHI Takahiro wrote:
> This patch adds auditing functions on entry to or exit from
> every system call invocation.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/kernel/entry.S | 3 +++
> arch/arm64/kernel/ptrace.c | 10 ++++++++++
> 3 files changed, 14 insertions(+)
>
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 720e70b..7468388 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -101,6 +101,7 @@ static inline struct thread_info *current_thread_info(void)
> #define TIF_NEED_RESCHED 1
> #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
> #define TIF_SYSCALL_TRACE 8
> +#define TIF_SYSCALL_AUDIT 9
> #define TIF_POLLING_NRFLAG 16
> #define TIF_MEMDIE 18 /* is terminating due to OOM killer */
> #define TIF_FREEZE 19
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 827cbad..83c4b29 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -630,6 +630,9 @@ el0_svc_naked: // compat entry point
> get_thread_info tsk
> ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
> tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
> +#ifdef CONFIG_AUDITSYSCALL
> + tbnz x16, #TIF_SYSCALL_AUDIT, __sys_trace // auditing syscalls?
> +#endif

Could we avoid the back-to-back tbnz instructions with a single mask? It's
not obvious that it will end up any better, but it would be good to know.

> adr lr, ret_fast_syscall // return address
> cmp scno, sc_nr // check upper syscall limit
> b.hs ni_sys
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6777a21..75a3f23 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -19,6 +19,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/audit.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> #include <linux/mm.h>
> @@ -38,6 +39,7 @@
> #include <asm/compat.h>
> #include <asm/debug-monitors.h>
> #include <asm/pgtable.h>
> +#include <asm/syscall.h>
> #include <asm/traps.h>
> #include <asm/system_misc.h>
>
> @@ -1064,6 +1066,14 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> {
> unsigned long saved_reg;
>
> + if (dir)
> + audit_syscall_exit(regs);
> + else
> + audit_syscall_entry(syscall_get_arch(current, regs),
> + (int)regs->syscallno,
> + regs->orig_x0, regs->regs[1],
> + regs->regs[2], regs->regs[3]);
> +

Do we really want to perform the audit checks before the tracehook calls?
Remember that the latter can rewrite all of the registers.

Will

2014-02-05 01:54:39

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v3 3/3] arm64: audit: Add audit hook in ptrace/syscall_trace

On 02/05/2014 02:31 AM, Will Deacon wrote:
> On Mon, Feb 03, 2014 at 06:56:30AM +0000, AKASHI Takahiro wrote:
>> This patch adds auditing functions on entry to or exit from
>> every system call invocation.
>>
>> Signed-off-by: AKASHI Takahiro <[email protected]>
>> ---
>> arch/arm64/include/asm/thread_info.h | 1 +
>> arch/arm64/kernel/entry.S | 3 +++
>> arch/arm64/kernel/ptrace.c | 10 ++++++++++
>> 3 files changed, 14 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
>> index 720e70b..7468388 100644
>> --- a/arch/arm64/include/asm/thread_info.h
>> +++ b/arch/arm64/include/asm/thread_info.h
>> @@ -101,6 +101,7 @@ static inline struct thread_info *current_thread_info(void)
>> #define TIF_NEED_RESCHED 1
>> #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
>> #define TIF_SYSCALL_TRACE 8
>> +#define TIF_SYSCALL_AUDIT 9
>> #define TIF_POLLING_NRFLAG 16
>> #define TIF_MEMDIE 18 /* is terminating due to OOM killer */
>> #define TIF_FREEZE 19
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 827cbad..83c4b29 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -630,6 +630,9 @@ el0_svc_naked: // compat entry point
>> get_thread_info tsk
>> ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
>> tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
>> +#ifdef CONFIG_AUDITSYSCALL
>> + tbnz x16, #TIF_SYSCALL_AUDIT, __sys_trace // auditing syscalls?
>> +#endif
>
> Could we avoid the back-to-back tbnz instructions with a single mask? It's
> not obvious that it will end up any better, but it would be good to know.

When first implementing ftrace support, TIF_SYSCALL_TRACEPOINT is defined as 10
and 'tst' instruction doesn't allow the following code:
tst x16, #(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_TRACEPOINT)

That is why I've used "back-to-back" tbnz since then, but now that I'm going to
submit ftrace, audit and later seccomp, I will replace it with:
#define TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE|TRACEPOINT|AUDIT|SECCOMP)

tst x16, #TIF_SYSCALL_WORK
b.ne __syscall_trace


>> adr lr, ret_fast_syscall // return address
>> cmp scno, sc_nr // check upper syscall limit
>> b.hs ni_sys
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index 6777a21..75a3f23 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -19,6 +19,7 @@
>> * along with this program. If not, see <http://www.gnu.org/licenses/>.
>> */
>>
>> +#include <linux/audit.h>
>> #include <linux/kernel.h>
>> #include <linux/sched.h>
>> #include <linux/mm.h>
>> @@ -38,6 +39,7 @@
>> #include <asm/compat.h>
>> #include <asm/debug-monitors.h>
>> #include <asm/pgtable.h>
>> +#include <asm/syscall.h>
>> #include <asm/traps.h>
>> #include <asm/system_misc.h>
>>
>> @@ -1064,6 +1066,14 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
>> {
>> unsigned long saved_reg;
>>
>> + if (dir)
>> + audit_syscall_exit(regs);
>> + else
>> + audit_syscall_entry(syscall_get_arch(current, regs),
>> + (int)regs->syscallno,
>> + regs->orig_x0, regs->regs[1],
>> + regs->regs[2], regs->regs[3]);
>> +
>
> Do we really want to perform the audit checks before the tracehook calls?
> Remember that the latter can rewrite all of the registers.

OK. I will change the code to make calls in the following order:
On entry,
*secure_computing
*tracehook_report_syscall(ENTER)
*trace_sys_enter
*audit_syscall_entry
On exit,
*audit_syscall_exit
*trace_sys_exit
*tracehook_report_syscall(EXIT)

The order here is the exact same as on x86, but such change might
decrease the readability in syscall_trace().

Thanks,
-Takahiro AKASHI

> Will
>

2014-02-07 10:08:20

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH] arm64: make a single hook to syscall_trace() for all syscall features

Currently syscall_trace() is called only for ptrace.
With additional TIF_xx flags introduced, it is now called in all the cases
of audit, ftrace and seccomp in addition to ptrace.
Those features will be implemented later, but it's safe to include them
now because they can not be turned on anyway.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/thread_info.h | 13 +++++++++++++
arch/arm64/kernel/entry.S | 5 +++--
arch/arm64/kernel/ptrace.c | 11 +++++------
3 files changed, 21 insertions(+), 8 deletions(-)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 720e70b..c3df797 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -91,6 +91,9 @@ static inline struct thread_info *current_thread_info(void)
/*
* thread information flags:
* TIF_SYSCALL_TRACE - syscall trace active
+ * TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
+ * TIF_SYSCALL_AUDIT - syscall auditing
+ * TIF_SECOMP - syscall secure computing
* TIF_SIGPENDING - signal pending
* TIF_NEED_RESCHED - rescheduling necessary
* TIF_NOTIFY_RESUME - callback before returning to user
@@ -101,6 +104,9 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NEED_RESCHED 1
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
#define TIF_SYSCALL_TRACE 8
+#define TIF_SYSCALL_AUDIT 9
+#define TIF_SYSCALL_TRACEPOINT 10
+#define TIF_SECCOMP 11
#define TIF_POLLING_NRFLAG 16
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
@@ -112,10 +118,17 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
+#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
+#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
+#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
+#define _TIF_SECCOMP (1 << TIF_SECCOMP)
#define _TIF_32BIT (1 << TIF_32BIT)

#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
_TIF_NOTIFY_RESUME)

+#define _TIF_WORK_SYSCALL (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+ _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
+
#endif /* __KERNEL__ */
#endif /* __ASM_THREAD_INFO_H */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 39ac630..c94b2ab 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -631,8 +631,9 @@ el0_svc_naked: // compat entry point
enable_irq

get_thread_info tsk
- ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
- tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
+ ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
+ tst x16, #_TIF_WORK_SYSCALL
+ b.ne __sys_trace
adr lr, ret_fast_syscall // return address
cmp scno, sc_nr // check upper syscall limit
b.hs ni_sys
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6a8928b..64ce39f 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1062,9 +1062,6 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
{
unsigned long saved_reg;

- if (!test_thread_flag(TIF_SYSCALL_TRACE))
- return regs->syscallno;
-
if (is_compat_task()) {
/* AArch32 uses ip (r12) for scratch */
saved_reg = regs->regs[12];
@@ -1078,10 +1075,12 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
regs->regs[7] = dir;
}

- if (dir)
+ if (dir) {
tracehook_report_syscall_exit(regs, 0);
- else if (tracehook_report_syscall_entry(regs))
- regs->syscallno = ~0UL;
+ } else {
+ if (tracehook_report_syscall_entry(regs))
+ regs->syscallno = ~0UL;
+ }

if (is_compat_task())
regs->regs[12] = saved_reg;
--
1.7.9.5

2014-02-07 10:10:34

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v4 0/3] arm64: Add audit support

This patchset adds system call audit support on arm64.
Both 32-bit (AUDIT_ARCH_ARM) and 64-bit tasks (AUDIT_ARCH_AARCH64)
are supported. Since arm64 has the exact same set of system calls
on LE and BE, we don't care about endianness (or more specifically
__AUDIT_ARCH_64BIT bit in AUDIT_ARCH_*).

There are some prerequisites for this patch to work correctly:
* "generic compat system call audit support" patch
* "correct a type mismatch in audit_syscall_exit()" patch
(already accepted and queued in 3.14)
* "Modify a set of system calls in audit class" patch
(already accepted and queued in 3.14)
* "__NR_* definitions for compat syscalls" patch from Catalin
* "make a single hook to syscall_trace() for all syscall features" patch
* userspace audit tool (v2.3.2 + my patch for arm64)

Please review them as well for better understandings.

This code was tested on both 32-bit and 64-bit LE userland
in the following two ways:
1) basic operations with auditctl/autrace
# auditctl -a exit,always -S openat -F path=/etc/inittab
# auditctl -a exit,always -F dir=/tmp -F perm=rw
# auditctl -a task,always
# autrace /bin/ls
by comparing output from autrace with one from strace

2) audit-test-code (+ my workarounds for arm/arm64)
by running "audit-tool", "filter" and "syscalls" test categories.

Changes v1 -> v2:
* Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
Please note that a required header, unistd_32.h, is automatically
generated from unistd32.h.
* Refer to regs->orig_x0 instead of regs->x0 as the first argument of
system call in audit_syscall_entry() [6/6]
* Include "Add regs_return_value() in syscall.h" patch [2/6],
which was not intentionally included in v1 because it could be added
by "kprobes support".

Changes v2 -> v3:
* Remove asm/audit.h.
See "generic compat syscall audit support" patch v4
* Remove endianness dependency, ie. AUDIT_ARCH_ARMEB/AARCH64EB.
* Remove kernel/syscalls/Makefile which was used to create unistd32.h.
See Catalin's "Add __NR_* definitions for compat syscalls" patch

Changes v3 -> v4:
* Modified to sync with the patch, "make a single hook to syscall_trace()
for all syscall features"


AKASHI Takahiro (3):
arm64: Add regs_return_value() in syscall.h
arm64: Add audit support
arm64: audit: Add audit hook in ptrace/syscall_trace

arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/ptrace.h | 5 +++++
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
arch/arm64/kernel/ptrace.c | 7 +++++++
include/uapi/linux/audit.h | 1 +
5 files changed, 29 insertions(+)

--
1.7.9.5

2014-02-07 10:10:46

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v4 1/3] arm64: Add regs_return_value() in syscall.h

This macro, regs_return_value, is used mainly for audit to record system
call's results, but may also be used in test_kprobes.c.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by Will Deacon <[email protected]>
---
arch/arm64/include/asm/ptrace.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 0e7fa49..5800ec1 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -134,6 +134,11 @@ struct pt_regs {
#define user_stack_pointer(regs) \
((regs)->sp)

+static inline unsigned long regs_return_value(struct pt_regs *regs)
+{
+ return regs->regs[0];
+}
+
/*
* Are the current registers suitable for user mode? (used to maintain
* security in signal handlers)
--
1.7.9.5

2014-02-07 10:10:52

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v4 2/3] arm64: Add audit support

On AArch64, audit is supported through generic lib/audit.c and
compat_audit.c, and so this patch adds arch specific definitions required.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by Will Deacon <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
include/uapi/linux/audit.h | 1 +
3 files changed, 17 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index dd4327f..a21455e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -25,6 +25,7 @@ config ARM64
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
select HARDIRQS_SW_RESEND
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_TRACEHOOK
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 70ba9d4..6900183 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -16,7 +16,9 @@
#ifndef __ASM_SYSCALL_H
#define __ASM_SYSCALL_H

+#include <linux/audit.h>
#include <linux/err.h>
+#include <asm/compat.h>


static inline int syscall_get_nr(struct task_struct *task,
@@ -104,4 +106,17 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->regs[i], args, n * sizeof(args[0]));
}

+/*
+ * We don't care about endianness (__AUDIT_ARCH_LE bit) here because
+ * AArch64 has the same system calls both on little- and big- endian.
+ */
+static inline int syscall_get_arch(struct task_struct *task,
+ struct pt_regs *regs)
+{
+ if (is_compat_thread(task_thread_info(task)))
+ return AUDIT_ARCH_ARM;
+
+ return AUDIT_ARCH_AARCH64;
+}
+
#endif /* __ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 624df43..aa86fab 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -333,6 +333,7 @@ enum {
/* distinguish syscall tables */
#define __AUDIT_ARCH_64BIT 0x80000000
#define __AUDIT_ARCH_LE 0x40000000
+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARMEB (EM_ARM)
--
1.7.9.5

2014-02-07 10:10:59

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v4 3/3] arm64: audit: Add audit hook in ptrace/syscall_trace

This patch adds auditing functions on entry to or exit from
every system call invocation.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/kernel/ptrace.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 64ce39f..8cdba09 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -19,6 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/audit.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/mm.h>
@@ -38,6 +39,7 @@
#include <asm/compat.h>
#include <asm/debug-monitors.h>
#include <asm/pgtable.h>
+#include <asm/syscall.h>
#include <asm/traps.h>
#include <asm/system_misc.h>

@@ -1076,10 +1078,15 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
}

if (dir) {
+ audit_syscall_exit(regs);
tracehook_report_syscall_exit(regs, 0);
} else {
if (tracehook_report_syscall_entry(regs))
regs->syscallno = ~0UL;
+ audit_syscall_entry(syscall_get_arch(current, regs),
+ (int)regs->syscallno,
+ regs->orig_x0, regs->regs[1],
+ regs->regs[2], regs->regs[3]);
}

if (is_compat_task())
--
1.7.9.5

2014-02-11 13:50:27

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH] arm64: make a single hook to syscall_trace() for all syscall features

On 14/02/07, AKASHI Takahiro wrote:
> Currently syscall_trace() is called only for ptrace.
> With additional TIF_xx flags introduced, it is now called in all the cases
> of audit, ftrace and seccomp in addition to ptrace.
> Those features will be implemented later, but it's safe to include them
> now because they can not be turned on anyway.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>

Acked-by: Richard Guy Briggs <[email protected]>

> ---
> arch/arm64/include/asm/thread_info.h | 13 +++++++++++++
> arch/arm64/kernel/entry.S | 5 +++--
> arch/arm64/kernel/ptrace.c | 11 +++++------
> 3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 720e70b..c3df797 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -91,6 +91,9 @@ static inline struct thread_info *current_thread_info(void)
> /*
> * thread information flags:
> * TIF_SYSCALL_TRACE - syscall trace active
> + * TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
> + * TIF_SYSCALL_AUDIT - syscall auditing
> + * TIF_SECOMP - syscall secure computing
> * TIF_SIGPENDING - signal pending
> * TIF_NEED_RESCHED - rescheduling necessary
> * TIF_NOTIFY_RESUME - callback before returning to user
> @@ -101,6 +104,9 @@ static inline struct thread_info *current_thread_info(void)
> #define TIF_NEED_RESCHED 1
> #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
> #define TIF_SYSCALL_TRACE 8
> +#define TIF_SYSCALL_AUDIT 9
> +#define TIF_SYSCALL_TRACEPOINT 10
> +#define TIF_SECCOMP 11
> #define TIF_POLLING_NRFLAG 16
> #define TIF_MEMDIE 18 /* is terminating due to OOM killer */
> #define TIF_FREEZE 19
> @@ -112,10 +118,17 @@ static inline struct thread_info *current_thread_info(void)
> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
> #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
> +#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
> +#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
> +#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
> +#define _TIF_SECCOMP (1 << TIF_SECCOMP)
> #define _TIF_32BIT (1 << TIF_32BIT)
>
> #define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
> _TIF_NOTIFY_RESUME)
>
> +#define _TIF_WORK_SYSCALL (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
> + _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
> +
> #endif /* __KERNEL__ */
> #endif /* __ASM_THREAD_INFO_H */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 39ac630..c94b2ab 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -631,8 +631,9 @@ el0_svc_naked: // compat entry point
> enable_irq
>
> get_thread_info tsk
> - ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
> - tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
> + ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
> + tst x16, #_TIF_WORK_SYSCALL
> + b.ne __sys_trace
> adr lr, ret_fast_syscall // return address
> cmp scno, sc_nr // check upper syscall limit
> b.hs ni_sys
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6a8928b..64ce39f 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1062,9 +1062,6 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> {
> unsigned long saved_reg;
>
> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
> - return regs->syscallno;
> -
> if (is_compat_task()) {
> /* AArch32 uses ip (r12) for scratch */
> saved_reg = regs->regs[12];
> @@ -1078,10 +1075,12 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> regs->regs[7] = dir;
> }
>
> - if (dir)
> + if (dir) {
> tracehook_report_syscall_exit(regs, 0);
> - else if (tracehook_report_syscall_entry(regs))
> - regs->syscallno = ~0UL;
> + } else {
> + if (tracehook_report_syscall_entry(regs))
> + regs->syscallno = ~0UL;
> + }
>
> if (is_compat_task())
> regs->regs[12] = saved_reg;
> --
> 1.7.9.5
>
> --
> Linux-audit mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-02-11 14:42:44

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v4 0/3] arm64: Add audit support

On 14/02/07, AKASHI Takahiro wrote:
> This patchset adds system call audit support on arm64.
> Both 32-bit (AUDIT_ARCH_ARM) and 64-bit tasks (AUDIT_ARCH_AARCH64)
> are supported. Since arm64 has the exact same set of system calls
> on LE and BE, we don't care about endianness (or more specifically
> __AUDIT_ARCH_64BIT bit in AUDIT_ARCH_*).
>
> There are some prerequisites for this patch to work correctly:
> * "generic compat system call audit support" patch
> * "correct a type mismatch in audit_syscall_exit()" patch
> (already accepted and queued in 3.14)
> * "Modify a set of system calls in audit class" patch
> (already accepted and queued in 3.14)
> * "__NR_* definitions for compat syscalls" patch from Catalin
> * "make a single hook to syscall_trace() for all syscall features" patch
> * userspace audit tool (v2.3.2 + my patch for arm64)
>
> Please review them as well for better understandings.
>
> This code was tested on both 32-bit and 64-bit LE userland
> in the following two ways:
> 1) basic operations with auditctl/autrace
> # auditctl -a exit,always -S openat -F path=/etc/inittab
> # auditctl -a exit,always -F dir=/tmp -F perm=rw
> # auditctl -a task,always
> # autrace /bin/ls
> by comparing output from autrace with one from strace
>
> 2) audit-test-code (+ my workarounds for arm/arm64)
> by running "audit-tool", "filter" and "syscalls" test categories.
>
> Changes v1 -> v2:
> * Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
> Please note that a required header, unistd_32.h, is automatically
> generated from unistd32.h.
> * Refer to regs->orig_x0 instead of regs->x0 as the first argument of
> system call in audit_syscall_entry() [6/6]
> * Include "Add regs_return_value() in syscall.h" patch [2/6],
> which was not intentionally included in v1 because it could be added
> by "kprobes support".
>
> Changes v2 -> v3:
> * Remove asm/audit.h.
> See "generic compat syscall audit support" patch v4
> * Remove endianness dependency, ie. AUDIT_ARCH_ARMEB/AARCH64EB.
> * Remove kernel/syscalls/Makefile which was used to create unistd32.h.
> See Catalin's "Add __NR_* definitions for compat syscalls" patch
>
> Changes v3 -> v4:
> * Modified to sync with the patch, "make a single hook to syscall_trace()
> for all syscall features"
>
>
> AKASHI Takahiro (3):
> arm64: Add regs_return_value() in syscall.h
> arm64: Add audit support
> arm64: audit: Add audit hook in ptrace/syscall_trace
>
> arch/arm64/Kconfig | 1 +
> arch/arm64/include/asm/ptrace.h | 5 +++++
> arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
> arch/arm64/kernel/ptrace.c | 7 +++++++
> include/uapi/linux/audit.h | 1 +
> 5 files changed, 29 insertions(+)

Compile and regression tested on: ppc s390 x86_64 ppc64 i686 s390x.

Acked-by: Richard Guy Briggs <[email protected]>

> --
> 1.7.9.5
>
> --
> Linux-audit mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-02-17 17:36:10

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH] arm64: make a single hook to syscall_trace() for all syscall features

On Fri, Feb 07, 2014 at 10:07:31AM +0000, AKASHI Takahiro wrote:
> Currently syscall_trace() is called only for ptrace.
> With additional TIF_xx flags introduced, it is now called in all the cases
> of audit, ftrace and seccomp in addition to ptrace.
> Those features will be implemented later, but it's safe to include them
> now because they can not be turned on anyway.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/arm64/include/asm/thread_info.h | 13 +++++++++++++
> arch/arm64/kernel/entry.S | 5 +++--
> arch/arm64/kernel/ptrace.c | 11 +++++------
> 3 files changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 720e70b..c3df797 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h

[...]

> +#define _TIF_WORK_SYSCALL (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
> + _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)

This is called _TIF_SYSCALL_WORK on arch/arm/, any reason not to follow the
naming convention here?

> #endif /* __KERNEL__ */
> #endif /* __ASM_THREAD_INFO_H */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 39ac630..c94b2ab 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -631,8 +631,9 @@ el0_svc_naked: // compat entry point
> enable_irq
>
> get_thread_info tsk
> - ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
> - tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
> + ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
> + tst x16, #_TIF_WORK_SYSCALL
> + b.ne __sys_trace
> adr lr, ret_fast_syscall // return address
> cmp scno, sc_nr // check upper syscall limit
> b.hs ni_sys
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6a8928b..64ce39f 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1062,9 +1062,6 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> {
> unsigned long saved_reg;
>
> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
> - return regs->syscallno;

This doesn't look right for things like audit (where we don't want to report
the syscall if only _TIF_SYSCALL_AUDIT is set, for example).

> if (is_compat_task()) {
> /* AArch32 uses ip (r12) for scratch */
> saved_reg = regs->regs[12];
> @@ -1078,10 +1075,12 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> regs->regs[7] = dir;
> }
>
> - if (dir)
> + if (dir) {
> tracehook_report_syscall_exit(regs, 0);
> - else if (tracehook_report_syscall_entry(regs))
> - regs->syscallno = ~0UL;
> + } else {
> + if (tracehook_report_syscall_entry(regs))
> + regs->syscallno = ~0UL;
> + }

This hunk doesn't do anything.

Will

2014-02-17 17:43:46

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v4 3/3] arm64: audit: Add audit hook in ptrace/syscall_trace

On Fri, Feb 07, 2014 at 10:10:03AM +0000, AKASHI Takahiro wrote:
> This patch adds auditing functions on entry to or exit from
> every system call invocation.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/arm64/kernel/ptrace.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 64ce39f..8cdba09 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -19,6 +19,7 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> */
>
> +#include <linux/audit.h>
> #include <linux/kernel.h>
> #include <linux/sched.h>
> #include <linux/mm.h>
> @@ -38,6 +39,7 @@
> #include <asm/compat.h>
> #include <asm/debug-monitors.h>
> #include <asm/pgtable.h>
> +#include <asm/syscall.h>
> #include <asm/traps.h>
> #include <asm/system_misc.h>
>
> @@ -1076,10 +1078,15 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> }
>
> if (dir) {
> + audit_syscall_exit(regs);
> tracehook_report_syscall_exit(regs, 0);
> } else {
> if (tracehook_report_syscall_entry(regs))
> regs->syscallno = ~0UL;
> + audit_syscall_entry(syscall_get_arch(current, regs),
> + (int)regs->syscallno,
> + regs->orig_x0, regs->regs[1],
> + regs->regs[2], regs->regs[3]);

Again, I don't think we should just lump tracehook and audit together like
this without checking the flags (see my reply to the previous patch series).

Will

2014-02-19 11:53:27

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH] arm64: make a single hook to syscall_trace() for all syscall features

Hi,

On 02/18/2014 02:35 AM, Will Deacon wrote:
> On Fri, Feb 07, 2014 at 10:07:31AM +0000, AKASHI Takahiro wrote:
>> Currently syscall_trace() is called only for ptrace.
>> With additional TIF_xx flags introduced, it is now called in all the cases
>> of audit, ftrace and seccomp in addition to ptrace.
>> Those features will be implemented later, but it's safe to include them
>> now because they can not be turned on anyway.
>>
>> Signed-off-by: AKASHI Takahiro <[email protected]>
>> ---
>> arch/arm64/include/asm/thread_info.h | 13 +++++++++++++
>> arch/arm64/kernel/entry.S | 5 +++--
>> arch/arm64/kernel/ptrace.c | 11 +++++------
>> 3 files changed, 21 insertions(+), 8 deletions(-)
>>
>> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
>> index 720e70b..c3df797 100644
>> --- a/arch/arm64/include/asm/thread_info.h
>> +++ b/arch/arm64/include/asm/thread_info.h
>
> [...]
>
>> +#define _TIF_WORK_SYSCALL (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
>> + _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
>
> This is called _TIF_SYSCALL_WORK on arch/arm/, any reason not to follow the
> naming convention here?

This is called _TIF_WORK_SYSCALL on arch/x86 :-)
That is the only reason, and so I don't have any objection to following arm
if you prefer it.

>> #endif /* __KERNEL__ */
>> #endif /* __ASM_THREAD_INFO_H */
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 39ac630..c94b2ab 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -631,8 +631,9 @@ el0_svc_naked: // compat entry point
>> enable_irq
>>
>> get_thread_info tsk
>> - ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
>> - tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
>> + ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
>> + tst x16, #_TIF_WORK_SYSCALL
>> + b.ne __sys_trace
>> adr lr, ret_fast_syscall // return address
>> cmp scno, sc_nr // check upper syscall limit
>> b.hs ni_sys
>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index 6a8928b..64ce39f 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -1062,9 +1062,6 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
>> {
>> unsigned long saved_reg;
>>
>> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
>> - return regs->syscallno;
>
> This doesn't look right for things like audit (where we don't want to report
> the syscall if only _TIF_SYSCALL_AUDIT is set, for example).

Yeah, it is my screwup.
I will add the guards against TIF_SYSCALL_TRACE (for ptrace),
TIF_SYSCALL_TRACEPOINT (for ftrace) and TIF_SYSCALL_AUDIT (for audit).

secure_computing() is protected in itself.

>> if (is_compat_task()) {
>> /* AArch32 uses ip (r12) for scratch */
>> saved_reg = regs->regs[12];
>> @@ -1078,10 +1075,12 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
>> regs->regs[7] = dir;
>> }
>>
>> - if (dir)
>> + if (dir) {
>> tracehook_report_syscall_exit(regs, 0);
>> - else if (tracehook_report_syscall_entry(regs))
>> - regs->syscallno = ~0UL;
>> + } else {
>> + if (tracehook_report_syscall_entry(regs))
>> + regs->syscallno = ~0UL;
>> + }
>
> This hunk doesn't do anything.

Well, this is just a change for future patches, but
I will remove it anyway due to the guards mentioned above.

-Takahiro AKASHI

> Will
>

2014-02-25 09:15:40

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 0/1] arm64: make a single hook to syscall_trace() for all syscall features

This patch makes it easy to add syscall related hooks, including ftrace,
audit and seccomp, in syscall_trace() later.
Those features will be implemented in separate patchsets, but it's safe to
check for all TIF_* now because they can not be turned on anyway.

Changes v1 -> v2:
* added a guard against TIF_SYSCALL_TRACE at tracehook_report_syscall_*()
* renamed _TIF_WORK_SYSCALL to _TIF_SYSCALL_WORK

AKASHI Takahiro (1):
arm64: make a single hook to syscall_trace() for all syscall features

arch/arm64/include/asm/thread_info.h | 13 ++++++++++
arch/arm64/kernel/entry.S | 5 ++--
arch/arm64/kernel/ptrace.c | 45 +++++++++++++++++-----------------
3 files changed, 38 insertions(+), 25 deletions(-)

--
1.7.9.5

2014-02-25 09:15:52

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 1/1] arm64: make a single hook to syscall_trace() for all syscall features

Currently syscall_trace() is called only for ptrace.
With additional TIF_xx flags defined, it is now called in all the cases
of audit, ftrace and seccomp in addition to ptrace.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by: Richard Guy Briggs <[email protected]>
---
arch/arm64/include/asm/thread_info.h | 13 ++++++++++
arch/arm64/kernel/entry.S | 5 ++--
arch/arm64/kernel/ptrace.c | 45 +++++++++++++++++-----------------
3 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 720e70b..0a8b2a9 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -91,6 +91,9 @@ static inline struct thread_info *current_thread_info(void)
/*
* thread information flags:
* TIF_SYSCALL_TRACE - syscall trace active
+ * TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
+ * TIF_SYSCALL_AUDIT - syscall auditing
+ * TIF_SECOMP - syscall secure computing
* TIF_SIGPENDING - signal pending
* TIF_NEED_RESCHED - rescheduling necessary
* TIF_NOTIFY_RESUME - callback before returning to user
@@ -101,6 +104,9 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NEED_RESCHED 1
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
#define TIF_SYSCALL_TRACE 8
+#define TIF_SYSCALL_AUDIT 9
+#define TIF_SYSCALL_TRACEPOINT 10
+#define TIF_SECCOMP 11
#define TIF_POLLING_NRFLAG 16
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
@@ -112,10 +118,17 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
+#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
+#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
+#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
+#define _TIF_SECCOMP (1 << TIF_SECCOMP)
#define _TIF_32BIT (1 << TIF_32BIT)

#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
_TIF_NOTIFY_RESUME)

+#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+ _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
+
#endif /* __KERNEL__ */
#endif /* __ASM_THREAD_INFO_H */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 0d7b789..6d613cd 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -630,8 +630,9 @@ el0_svc_naked: // compat entry point
enable_irq

get_thread_info tsk
- ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
- tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
+ ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
+ tst x16, #_TIF_SYSCALL_WORK
+ b.ne __sys_trace
adr lr, ret_fast_syscall // return address
cmp scno, sc_nr // check upper syscall limit
b.hs ni_sys
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6a8928b..c70133e 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1062,31 +1062,30 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
{
unsigned long saved_reg;

- if (!test_thread_flag(TIF_SYSCALL_TRACE))
- return regs->syscallno;
-
- if (is_compat_task()) {
- /* AArch32 uses ip (r12) for scratch */
- saved_reg = regs->regs[12];
- regs->regs[12] = dir;
- } else {
- /*
- * Save X7. X7 is used to denote syscall entry/exit:
- * X7 = 0 -> entry, = 1 -> exit
- */
- saved_reg = regs->regs[7];
- regs->regs[7] = dir;
- }
+ if (test_thread_flag(TIF_SYSCALL_TRACE)) {
+ if (is_compat_task()) {
+ /* AArch32 uses ip (r12) for scratch */
+ saved_reg = regs->regs[12];
+ regs->regs[12] = dir;
+ } else {
+ /*
+ * Save X7. X7 is used to denote syscall entry/exit:
+ * X7 = 0 -> entry, = 1 -> exit
+ */
+ saved_reg = regs->regs[7];
+ regs->regs[7] = dir;
+ }

- if (dir)
- tracehook_report_syscall_exit(regs, 0);
- else if (tracehook_report_syscall_entry(regs))
- regs->syscallno = ~0UL;
+ if (dir)
+ tracehook_report_syscall_exit(regs, 0);
+ else if (tracehook_report_syscall_entry(regs))
+ regs->syscallno = ~0UL;

- if (is_compat_task())
- regs->regs[12] = saved_reg;
- else
- regs->regs[7] = saved_reg;
+ if (is_compat_task())
+ regs->regs[12] = saved_reg;
+ else
+ regs->regs[7] = saved_reg;
+ }

return regs->syscallno;
}
--
1.7.9.5

2014-02-25 09:17:29

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 0/1] audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

Currently AUDITSYSCALL has a long list of architecture depencency:
depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML ||
SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
The purpose of this patch is to replace it with HAVE_ARCH_AUDITSYSCALL
for simplicity.

Changes v1 -> v2:
* rebased to 3.14-rcX, and so added a change on ALPHA

AKASHI Takahiro (1):
audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

arch/alpha/Kconfig | 1 +
arch/arm/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/parisc/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/um/Kconfig.common | 1 +
arch/x86/Kconfig | 1 +
init/Kconfig | 5 ++++-
11 files changed, 14 insertions(+), 1 deletion(-)

--
1.7.9.5

2014-02-25 09:17:44

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v2 1/1] audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

Currently AUDITSYSCALL has a long list of architecture depencency:
depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML ||
SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
The purpose of this patch is to replace it with HAVE_ARCH_AUDITSYSCALL
for simplicity.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/alpha/Kconfig | 1 +
arch/arm/Kconfig | 1 +
arch/ia64/Kconfig | 1 +
arch/parisc/Kconfig | 1 +
arch/powerpc/Kconfig | 1 +
arch/s390/Kconfig | 1 +
arch/sh/Kconfig | 1 +
arch/sparc/Kconfig | 1 +
arch/um/Kconfig.common | 1 +
arch/x86/Kconfig | 1 +
init/Kconfig | 5 ++++-
11 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index f6c6b34..b7ff9a3 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -22,6 +22,7 @@ config ALPHA
select GENERIC_SMP_IDLE_THREAD
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_MOD_ARCH_SPECIFIC
select MODULES_USE_ELF_RELA
select ODD_RT_SIGACTION
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index e254198..ca79340 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -24,6 +24,7 @@ config ARM
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select HARDIRQS_SW_RESEND
+ select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
select HAVE_ARCH_KGDB
select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 0c8e553..5409bf4 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -44,6 +44,7 @@ config IA64
select HAVE_MOD_ARCH_SPECIFIC
select MODULES_USE_ELF_RELA
select ARCH_USE_CMPXCHG_LOCKREF
+ select HAVE_ARCH_AUDITSYSCALL
default y
help
The Itanium Processor Family is Intel's 64-bit successor to
diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index bb2a8ec..1faefed 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -28,6 +28,7 @@ config PARISC
select CLONE_BACKWARDS
select TTY # Needed for pdc_cons.c
select HAVE_DEBUG_STACKOVERFLOW
+ select HAVE_ARCH_AUDITSYSCALL

help
The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 957bf34..7b3b8fe 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -141,6 +141,7 @@ config PPC
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_IRQ_EXIT_ON_IRQ_STACK
select ARCH_USE_CMPXCHG_LOCKREF if PPC64
+ select HAVE_ARCH_AUDITSYSCALL

config GENERIC_CSUM
def_bool CPU_LITTLE_ENDIAN
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 65a0775..1b58568 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -103,6 +103,7 @@ config S390
select GENERIC_SMP_IDLE_THREAD
select GENERIC_TIME_VSYSCALL
select HAVE_ALIGNED_STRUCT_PAGE if SLUB
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL if !MARCH_G5
select HAVE_ARCH_SECCOMP_FILTER
select HAVE_ARCH_TRACEHOOK
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 6357710..4addd87 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -42,6 +42,7 @@ config SUPERH
select MODULES_USE_ELF_RELA
select OLD_SIGSUSPEND
select OLD_SIGACTION
+ select HAVE_ARCH_AUDITSYSCALL
help
The SuperH is a RISC processor targeted for use in embedded systems
and consumer electronics; it was also used in the Sega Dreamcast
diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
index c51efdc..9c74d6b 100644
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -77,6 +77,7 @@ config SPARC64
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select HAVE_C_RECORDMCOUNT
select NO_BOOTMEM
+ select HAVE_ARCH_AUDITSYSCALL

config ARCH_DEFCONFIG
string
diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
index 21ca44c..6915d28 100644
--- a/arch/um/Kconfig.common
+++ b/arch/um/Kconfig.common
@@ -1,6 +1,7 @@
config UML
bool
default y
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_UID16
select GENERIC_IRQ_SHOW
select GENERIC_CPU_DEVICES
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..2938365 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
select HAVE_DEBUG_STACKOVERFLOW
select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
select HAVE_CC_STACKPROTECTOR
+ select HAVE_ARCH_AUDITSYSCALL

config INSTRUCTION_DECODER
def_bool y
diff --git a/init/Kconfig b/init/Kconfig
index 009a797..d4ec53d 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -282,9 +282,12 @@ config AUDIT
logging of avc messages output). Does not do system-call
auditing without CONFIG_AUDITSYSCALL.

+config HAVE_ARCH_AUDITSYSCALL
+ bool
+
config AUDITSYSCALL
bool "Enable system-call auditing support"
- depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
+ depends on AUDIT && HAVE_ARCH_AUDITSYSCALL
default y if SECURITY_SELINUX
help
Enable low-overhead system-call auditing infrastructure that
--
1.7.9.5

2014-02-25 09:19:21

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v5 0/3] arm64: Add audit support

This patchset adds system call audit support on arm64.
Both 32-bit (AUDIT_ARCH_ARM) and 64-bit tasks (AUDIT_ARCH_AARCH64)
are supported. Since arm64 has the exact same set of system calls
on LE and BE, we don't care about endianness (or more specifically
__AUDIT_ARCH_64BIT bit in AUDIT_ARCH_*).

There are some prerequisites for this patch to work correctly:
* "audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL" patch
* "generic compat system call audit support" patch
* "__NR_* definitions for compat syscalls" patch from Catalin
* "make a single hook to syscall_trace() for all syscall features" patch
* userspace audit tool (v2.3.2 + my patch for arm64)

Please review them as well for better understandings.

This code was tested on both 32-bit and 64-bit LE userland
in the following two ways:
1) basic operations with auditctl/autrace
# auditctl -a exit,always -S openat -F path=/etc/inittab
# auditctl -a exit,always -F dir=/tmp -F perm=rw
# auditctl -a task,always
# autrace /bin/ls
by comparing output from autrace with one from strace

2) audit-test-code (+ my workarounds for arm/arm64)
by running "audit-tool", "filter" and "syscalls" test categories.

Changes v1 -> v2:
* Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
Please note that a required header, unistd_32.h, is automatically
generated from unistd32.h.
* Refer to regs->orig_x0 instead of regs->x0 as the first argument of
system call in audit_syscall_entry() [6/6]
* Include "Add regs_return_value() in syscall.h" patch [2/6],
which was not intentionally included in v1 because it could be added
by "kprobes support".

Changes v2 -> v3:
* Remove asm/audit.h.
See "generic compat syscall audit support" patch v4
* Remove endianness dependency, ie. AUDIT_ARCH_ARMEB/AARCH64EB.
* Remove kernel/syscalls/Makefile which was used to create unistd32.h.
See Catalin's "Add __NR_* definitions for compat syscalls" patch

Changes v3 -> v4:
* Modified to sync with the patch, "make a single hook to syscall_trace()
for all syscall features"
* aligned with "audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL" patch

Changes v4 -> v5:
* rebased to 3.14-rcX
* added a guard against TIF_SYSCALL_AUDIT [3/3]
* aligned with the change in "arm64: make a single hook to syscall_trace()
for all syscall features" v2 [3/3]

AKASHI Takahiro (3):
arm64: Add regs_return_value() in syscall.h
arm64: Add audit support
arm64: audit: Add audit hook in ptrace/syscall_trace

arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/ptrace.h | 5 +++++
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
arch/arm64/kernel/ptrace.c | 11 +++++++++++
include/uapi/linux/audit.h | 1 +
5 files changed, 33 insertions(+)

--
1.7.9.5

2014-02-25 09:19:32

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v5 1/3] arm64: Add regs_return_value() in syscall.h

This macro, regs_return_value, is used mainly for audit to record system
call's results, but may also be used in test_kprobes.c.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by Will Deacon <[email protected]>
Acked-by: Richard Guy Briggs <[email protected]>
---
arch/arm64/include/asm/ptrace.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 0e7fa49..5800ec1 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -134,6 +134,11 @@ struct pt_regs {
#define user_stack_pointer(regs) \
((regs)->sp)

+static inline unsigned long regs_return_value(struct pt_regs *regs)
+{
+ return regs->regs[0];
+}
+
/*
* Are the current registers suitable for user mode? (used to maintain
* security in signal handlers)
--
1.7.9.5

2014-02-25 09:19:43

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v5 2/3] arm64: Add audit support

On AArch64, audit is supported through generic lib/audit.c and
compat_audit.c, and so this patch adds arch specific definitions required.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by Will Deacon <[email protected]>
Acked-by: Richard Guy Briggs <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
include/uapi/linux/audit.h | 1 +
3 files changed, 17 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 27bbcfc..aa47548 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -25,6 +25,7 @@ config ARM64
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
select HARDIRQS_SW_RESEND
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_TRACEHOOK
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 70ba9d4..6900183 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -16,7 +16,9 @@
#ifndef __ASM_SYSCALL_H
#define __ASM_SYSCALL_H

+#include <linux/audit.h>
#include <linux/err.h>
+#include <asm/compat.h>


static inline int syscall_get_nr(struct task_struct *task,
@@ -104,4 +106,17 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->regs[i], args, n * sizeof(args[0]));
}

+/*
+ * We don't care about endianness (__AUDIT_ARCH_LE bit) here because
+ * AArch64 has the same system calls both on little- and big- endian.
+ */
+static inline int syscall_get_arch(struct task_struct *task,
+ struct pt_regs *regs)
+{
+ if (is_compat_thread(task_thread_info(task)))
+ return AUDIT_ARCH_ARM;
+
+ return AUDIT_ARCH_AARCH64;
+}
+
#endif /* __ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 624df43..aa86fab 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -333,6 +333,7 @@ enum {
/* distinguish syscall tables */
#define __AUDIT_ARCH_64BIT 0x80000000
#define __AUDIT_ARCH_LE 0x40000000
+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARMEB (EM_ARM)
--
1.7.9.5

2014-02-25 09:19:59

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v5 3/3] arm64: audit: Add audit hook in ptrace/syscall_trace

This patch adds auditing functions on entry to or exit from
every system call invocation.

Signed-off-by: AKASHI Takahiro <[email protected]>
Acked-by: Richard Guy Briggs <[email protected]>
---
arch/arm64/kernel/ptrace.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index c70133e..d4ce70e 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -19,6 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/audit.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/mm.h>
@@ -38,6 +39,7 @@
#include <asm/compat.h>
#include <asm/debug-monitors.h>
#include <asm/pgtable.h>
+#include <asm/syscall.h>
#include <asm/traps.h>
#include <asm/system_misc.h>

@@ -1062,6 +1064,9 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
{
unsigned long saved_reg;

+ if (dir && test_thread_flag(TIF_SYSCALL_AUDIT))
+ audit_syscall_exit(regs);
+
if (test_thread_flag(TIF_SYSCALL_TRACE)) {
if (is_compat_task()) {
/* AArch32 uses ip (r12) for scratch */
@@ -1087,5 +1092,11 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
regs->regs[7] = saved_reg;
}

+ if (!dir && test_thread_flag(TIF_SYSCALL_AUDIT))
+ audit_syscall_entry(syscall_get_arch(current, regs),
+ (int)regs->syscallno,
+ regs->orig_x0, regs->regs[1],
+ regs->regs[2], regs->regs[3]);
+
return regs->syscallno;
}
--
1.7.9.5

2014-02-25 15:00:49

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] arm64: make a single hook to syscall_trace() for all syscall features

On Tue, Feb 25, 2014 at 09:14:43AM +0000, AKASHI Takahiro wrote:
> Currently syscall_trace() is called only for ptrace.
> With additional TIF_xx flags defined, it is now called in all the cases
> of audit, ftrace and seccomp in addition to ptrace.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> Acked-by: Richard Guy Briggs <[email protected]>
> ---
> arch/arm64/include/asm/thread_info.h | 13 ++++++++++
> arch/arm64/kernel/entry.S | 5 ++--
> arch/arm64/kernel/ptrace.c | 45 +++++++++++++++++-----------------
> 3 files changed, 38 insertions(+), 25 deletions(-)
>
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index 720e70b..0a8b2a9 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -91,6 +91,9 @@ static inline struct thread_info *current_thread_info(void)
> /*
> * thread information flags:
> * TIF_SYSCALL_TRACE - syscall trace active
> + * TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
> + * TIF_SYSCALL_AUDIT - syscall auditing
> + * TIF_SECOMP - syscall secure computing
> * TIF_SIGPENDING - signal pending
> * TIF_NEED_RESCHED - rescheduling necessary
> * TIF_NOTIFY_RESUME - callback before returning to user
> @@ -101,6 +104,9 @@ static inline struct thread_info *current_thread_info(void)
> #define TIF_NEED_RESCHED 1
> #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
> #define TIF_SYSCALL_TRACE 8
> +#define TIF_SYSCALL_AUDIT 9
> +#define TIF_SYSCALL_TRACEPOINT 10
> +#define TIF_SECCOMP 11
> #define TIF_POLLING_NRFLAG 16
> #define TIF_MEMDIE 18 /* is terminating due to OOM killer */
> #define TIF_FREEZE 19
> @@ -112,10 +118,17 @@ static inline struct thread_info *current_thread_info(void)
> #define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
> #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
> #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
> +#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
> +#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
> +#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
> +#define _TIF_SECCOMP (1 << TIF_SECCOMP)
> #define _TIF_32BIT (1 << TIF_32BIT)
>
> #define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
> _TIF_NOTIFY_RESUME)
>
> +#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
> + _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
> +
> #endif /* __KERNEL__ */
> #endif /* __ASM_THREAD_INFO_H */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 0d7b789..6d613cd 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -630,8 +630,9 @@ el0_svc_naked: // compat entry point
> enable_irq
>
> get_thread_info tsk
> - ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
> - tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
> + ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
> + tst x16, #_TIF_SYSCALL_WORK
> + b.ne __sys_trace
> adr lr, ret_fast_syscall // return address
> cmp scno, sc_nr // check upper syscall limit
> b.hs ni_sys

All looks fine up to here.

> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index 6a8928b..c70133e 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1062,31 +1062,30 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> {
> unsigned long saved_reg;
>
> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
> - return regs->syscallno;
> -
> - if (is_compat_task()) {
> - /* AArch32 uses ip (r12) for scratch */
> - saved_reg = regs->regs[12];
> - regs->regs[12] = dir;
> - } else {
> - /*
> - * Save X7. X7 is used to denote syscall entry/exit:
> - * X7 = 0 -> entry, = 1 -> exit
> - */
> - saved_reg = regs->regs[7];
> - regs->regs[7] = dir;
> - }
> + if (test_thread_flag(TIF_SYSCALL_TRACE)) {
> + if (is_compat_task()) {
> + /* AArch32 uses ip (r12) for scratch */
> + saved_reg = regs->regs[12];
> + regs->regs[12] = dir;
> + } else {
> + /*
> + * Save X7. X7 is used to denote syscall entry/exit:
> + * X7 = 0 -> entry, = 1 -> exit
> + */
> + saved_reg = regs->regs[7];
> + regs->regs[7] = dir;
> + }
>
> - if (dir)
> - tracehook_report_syscall_exit(regs, 0);
> - else if (tracehook_report_syscall_entry(regs))
> - regs->syscallno = ~0UL;
> + if (dir)
> + tracehook_report_syscall_exit(regs, 0);
> + else if (tracehook_report_syscall_entry(regs))
> + regs->syscallno = ~0UL;
>
> - if (is_compat_task())
> - regs->regs[12] = saved_reg;
> - else
> - regs->regs[7] = saved_reg;
> + if (is_compat_task())
> + regs->regs[12] = saved_reg;
> + else
> + regs->regs[7] = saved_reg;
> + }

Aren't these changes (to ptrace.c) just a giant NOP?

Will

2014-02-25 15:26:28

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

On 14/02/25, AKASHI Takahiro wrote:
> Currently AUDITSYSCALL has a long list of architecture depencency:
> depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML ||
> SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
> The purpose of this patch is to replace it with HAVE_ARCH_AUDITSYSCALL
> for simplicity.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>

Acked-by: Richard Guy Briggs <[email protected]>

> ---
> arch/alpha/Kconfig | 1 +
> arch/arm/Kconfig | 1 +
> arch/ia64/Kconfig | 1 +
> arch/parisc/Kconfig | 1 +
> arch/powerpc/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/sh/Kconfig | 1 +
> arch/sparc/Kconfig | 1 +
> arch/um/Kconfig.common | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 5 ++++-
> 11 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
> index f6c6b34..b7ff9a3 100644
> --- a/arch/alpha/Kconfig
> +++ b/arch/alpha/Kconfig
> @@ -22,6 +22,7 @@ config ALPHA
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> + select HAVE_ARCH_AUDITSYSCALL
> select HAVE_MOD_ARCH_SPECIFIC
> select MODULES_USE_ELF_RELA
> select ODD_RT_SIGACTION
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e254198..ca79340 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -24,6 +24,7 @@ config ARM
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> select HARDIRQS_SW_RESEND
> + select HAVE_ARCH_AUDITSYSCALL if (AEABI && !OABI_COMPAT)
> select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL
> select HAVE_ARCH_KGDB
> select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index 0c8e553..5409bf4 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -44,6 +44,7 @@ config IA64
> select HAVE_MOD_ARCH_SPECIFIC
> select MODULES_USE_ELF_RELA
> select ARCH_USE_CMPXCHG_LOCKREF
> + select HAVE_ARCH_AUDITSYSCALL
> default y
> help
> The Itanium Processor Family is Intel's 64-bit successor to
> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> index bb2a8ec..1faefed 100644
> --- a/arch/parisc/Kconfig
> +++ b/arch/parisc/Kconfig
> @@ -28,6 +28,7 @@ config PARISC
> select CLONE_BACKWARDS
> select TTY # Needed for pdc_cons.c
> select HAVE_DEBUG_STACKOVERFLOW
> + select HAVE_ARCH_AUDITSYSCALL
>
> help
> The PA-RISC microprocessor is designed by Hewlett-Packard and used
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 957bf34..7b3b8fe 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -141,6 +141,7 @@ config PPC
> select HAVE_DEBUG_STACKOVERFLOW
> select HAVE_IRQ_EXIT_ON_IRQ_STACK
> select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> + select HAVE_ARCH_AUDITSYSCALL
>
> config GENERIC_CSUM
> def_bool CPU_LITTLE_ENDIAN
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 65a0775..1b58568 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -103,6 +103,7 @@ config S390
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_TIME_VSYSCALL
> select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> + select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_JUMP_LABEL if !MARCH_G5
> select HAVE_ARCH_SECCOMP_FILTER
> select HAVE_ARCH_TRACEHOOK
> diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
> index 6357710..4addd87 100644
> --- a/arch/sh/Kconfig
> +++ b/arch/sh/Kconfig
> @@ -42,6 +42,7 @@ config SUPERH
> select MODULES_USE_ELF_RELA
> select OLD_SIGSUSPEND
> select OLD_SIGACTION
> + select HAVE_ARCH_AUDITSYSCALL
> help
> The SuperH is a RISC processor targeted for use in embedded systems
> and consumer electronics; it was also used in the Sega Dreamcast
> diff --git a/arch/sparc/Kconfig b/arch/sparc/Kconfig
> index c51efdc..9c74d6b 100644
> --- a/arch/sparc/Kconfig
> +++ b/arch/sparc/Kconfig
> @@ -77,6 +77,7 @@ config SPARC64
> select ARCH_HAVE_NMI_SAFE_CMPXCHG
> select HAVE_C_RECORDMCOUNT
> select NO_BOOTMEM
> + select HAVE_ARCH_AUDITSYSCALL
>
> config ARCH_DEFCONFIG
> string
> diff --git a/arch/um/Kconfig.common b/arch/um/Kconfig.common
> index 21ca44c..6915d28 100644
> --- a/arch/um/Kconfig.common
> +++ b/arch/um/Kconfig.common
> @@ -1,6 +1,7 @@
> config UML
> bool
> default y
> + select HAVE_ARCH_AUDITSYSCALL
> select HAVE_UID16
> select GENERIC_IRQ_SHOW
> select GENERIC_CPU_DEVICES
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0af5250..2938365 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -127,6 +127,7 @@ config X86
> select HAVE_DEBUG_STACKOVERFLOW
> select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
> select HAVE_CC_STACKPROTECTOR
> + select HAVE_ARCH_AUDITSYSCALL
>
> config INSTRUCTION_DECODER
> def_bool y
> diff --git a/init/Kconfig b/init/Kconfig
> index 009a797..d4ec53d 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -282,9 +282,12 @@ config AUDIT
> logging of avc messages output). Does not do system-call
> auditing without CONFIG_AUDITSYSCALL.
>
> +config HAVE_ARCH_AUDITSYSCALL
> + bool
> +
> config AUDITSYSCALL
> bool "Enable system-call auditing support"
> - depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML || SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
> + depends on AUDIT && HAVE_ARCH_AUDITSYSCALL
> default y if SECURITY_SELINUX
> help
> Enable low-overhead system-call auditing infrastructure that
> --
> 1.7.9.5
>

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

2014-02-25 17:40:40

by Matt Turner

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

On Tue, Feb 25, 2014 at 1:16 AM, AKASHI Takahiro
<[email protected]> wrote:
> Currently AUDITSYSCALL has a long list of architecture depencency:
> depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML ||
> SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
> The purpose of this patch is to replace it with HAVE_ARCH_AUDITSYSCALL
> for simplicity.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/alpha/Kconfig | 1 +
> arch/arm/Kconfig | 1 +
> arch/ia64/Kconfig | 1 +
> arch/parisc/Kconfig | 1 +
> arch/powerpc/Kconfig | 1 +
> arch/s390/Kconfig | 1 +
> arch/sh/Kconfig | 1 +
> arch/sparc/Kconfig | 1 +
> arch/um/Kconfig.common | 1 +
> arch/x86/Kconfig | 1 +
> init/Kconfig | 5 ++++-
> 11 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
> index f6c6b34..b7ff9a3 100644
> --- a/arch/alpha/Kconfig
> +++ b/arch/alpha/Kconfig
> @@ -22,6 +22,7 @@ config ALPHA
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> + select HAVE_ARCH_AUDITSYSCALL
> select HAVE_MOD_ARCH_SPECIFIC
> select MODULES_USE_ELF_RELA
> select ODD_RT_SIGACTION

Thanks.

Acked-by: Matt Turner <[email protected]>

2014-02-26 02:00:30

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] arm64: make a single hook to syscall_trace() for all syscall features

On 02/26/2014 12:00 AM, Will Deacon wrote:
> On Tue, Feb 25, 2014 at 09:14:43AM +0000, AKASHI Takahiro wrote:
>> Currently syscall_trace() is called only for ptrace.
>> With additional TIF_xx flags defined, it is now called in all the cases
>> of audit, ftrace and seccomp in addition to ptrace.
>>
>> Signed-off-by: AKASHI Takahiro <[email protected]>
>> Acked-by: Richard Guy Briggs <[email protected]>
>> ---
>> arch/arm64/include/asm/thread_info.h | 13 ++++++++++
>> arch/arm64/kernel/entry.S | 5 ++--
>> arch/arm64/kernel/ptrace.c | 45 +++++++++++++++++-----------------
>> 3 files changed, 38 insertions(+), 25 deletions(-)

[...]

>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>> index 6a8928b..c70133e 100644
>> --- a/arch/arm64/kernel/ptrace.c
>> +++ b/arch/arm64/kernel/ptrace.c
>> @@ -1062,31 +1062,30 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
>> {
>> unsigned long saved_reg;
>>
>> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
>> - return regs->syscallno;
>> -
>> - if (is_compat_task()) {
>> - /* AArch32 uses ip (r12) for scratch */
>> - saved_reg = regs->regs[12];
>> - regs->regs[12] = dir;
>> - } else {
>> - /*
>> - * Save X7. X7 is used to denote syscall entry/exit:
>> - * X7 = 0 -> entry, = 1 -> exit
>> - */
>> - saved_reg = regs->regs[7];
>> - regs->regs[7] = dir;
>> - }
>> + if (test_thread_flag(TIF_SYSCALL_TRACE)) {
>> + if (is_compat_task()) {
>> + /* AArch32 uses ip (r12) for scratch */
>> + saved_reg = regs->regs[12];
>> + regs->regs[12] = dir;
>> + } else {
>> + /*
>> + * Save X7. X7 is used to denote syscall entry/exit:
>> + * X7 = 0 -> entry, = 1 -> exit
>> + */
>> + saved_reg = regs->regs[7];
>> + regs->regs[7] = dir;
>> + }
>>
>> - if (dir)
>> - tracehook_report_syscall_exit(regs, 0);
>> - else if (tracehook_report_syscall_entry(regs))
>> - regs->syscallno = ~0UL;
>> + if (dir)
>> + tracehook_report_syscall_exit(regs, 0);
>> + else if (tracehook_report_syscall_entry(regs))
>> + regs->syscallno = ~0UL;
>>
>> - if (is_compat_task())
>> - regs->regs[12] = saved_reg;
>> - else
>> - regs->regs[7] = saved_reg;
>> + if (is_compat_task())
>> + regs->regs[12] = saved_reg;
>> + else
>> + regs->regs[7] = saved_reg;
>> + }
>
> Aren't these changes (to ptrace.c) just a giant NOP?

Umm, the purpose of this big "if" is to run the code only if TIF_SYSCALL_TRACE is set,
and to make it easy to add additional hooks, audit and ftrace, around tracehook_report_*()
later on.

-Takahiro AKASHI

> Will
>

2014-02-26 11:26:28

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] arm64: make a single hook to syscall_trace() for all syscall features

On Wed, Feb 26, 2014 at 02:00:19AM +0000, AKASHI Takahiro wrote:
> On 02/26/2014 12:00 AM, Will Deacon wrote:
> > On Tue, Feb 25, 2014 at 09:14:43AM +0000, AKASHI Takahiro wrote:
> >> Currently syscall_trace() is called only for ptrace.
> >> With additional TIF_xx flags defined, it is now called in all the cases
> >> of audit, ftrace and seccomp in addition to ptrace.
> >>
> >> Signed-off-by: AKASHI Takahiro <[email protected]>
> >> Acked-by: Richard Guy Briggs <[email protected]>
> >> ---
> >> arch/arm64/include/asm/thread_info.h | 13 ++++++++++
> >> arch/arm64/kernel/entry.S | 5 ++--
> >> arch/arm64/kernel/ptrace.c | 45 +++++++++++++++++-----------------
> >> 3 files changed, 38 insertions(+), 25 deletions(-)
>
> [...]
>
> >> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> >> index 6a8928b..c70133e 100644
> >> --- a/arch/arm64/kernel/ptrace.c
> >> +++ b/arch/arm64/kernel/ptrace.c
> >> @@ -1062,31 +1062,30 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
> >> {
> >> unsigned long saved_reg;
> >>
> >> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
> >> - return regs->syscallno;
> >> -
> >> - if (is_compat_task()) {
> >> - /* AArch32 uses ip (r12) for scratch */
> >> - saved_reg = regs->regs[12];
> >> - regs->regs[12] = dir;
> >> - } else {
> >> - /*
> >> - * Save X7. X7 is used to denote syscall entry/exit:
> >> - * X7 = 0 -> entry, = 1 -> exit
> >> - */
> >> - saved_reg = regs->regs[7];
> >> - regs->regs[7] = dir;
> >> - }
> >> + if (test_thread_flag(TIF_SYSCALL_TRACE)) {
> >> + if (is_compat_task()) {
> >> + /* AArch32 uses ip (r12) for scratch */
> >> + saved_reg = regs->regs[12];
> >> + regs->regs[12] = dir;
> >> + } else {
> >> + /*
> >> + * Save X7. X7 is used to denote syscall entry/exit:
> >> + * X7 = 0 -> entry, = 1 -> exit
> >> + */
> >> + saved_reg = regs->regs[7];
> >> + regs->regs[7] = dir;
> >> + }
> >>
> >> - if (dir)
> >> - tracehook_report_syscall_exit(regs, 0);
> >> - else if (tracehook_report_syscall_entry(regs))
> >> - regs->syscallno = ~0UL;
> >> + if (dir)
> >> + tracehook_report_syscall_exit(regs, 0);
> >> + else if (tracehook_report_syscall_entry(regs))
> >> + regs->syscallno = ~0UL;
> >>
> >> - if (is_compat_task())
> >> - regs->regs[12] = saved_reg;
> >> - else
> >> - regs->regs[7] = saved_reg;
> >> + if (is_compat_task())
> >> + regs->regs[12] = saved_reg;
> >> + else
> >> + regs->regs[7] = saved_reg;
> >> + }
> >
> > Aren't these changes (to ptrace.c) just a giant NOP?
>
> Umm, the purpose of this big "if" is to run the code only if TIF_SYSCALL_TRACE is set,
> and to make it easy to add additional hooks, audit and ftrace, around tracehook_report_*()
> later on.

The existing code already checks TIF_SYSCALL_TRACE. I'd rather you added
this new code when it's actually nedded (e.g. when adding audit on top).

Will

2014-02-26 12:31:43

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL

On Tue, 2014-02-25 at 18:16 +0900, AKASHI Takahiro wrote:
> Currently AUDITSYSCALL has a long list of architecture depencency:
> depends on AUDIT && (X86 || PARISC || PPC || S390 || IA64 || UML ||
> SPARC64 || SUPERH || (ARM && AEABI && !OABI_COMPAT) || ALPHA)
> The purpose of this patch is to replace it with HAVE_ARCH_AUDITSYSCALL
> for simplicity.
>
> Signed-off-by: AKASHI Takahiro <[email protected]>
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 957bf34..7b3b8fe 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -141,6 +141,7 @@ config PPC
> select HAVE_DEBUG_STACKOVERFLOW
> select HAVE_IRQ_EXIT_ON_IRQ_STACK
> select ARCH_USE_CMPXCHG_LOCKREF if PPC64
> + select HAVE_ARCH_AUDITSYSCALL
>
> config GENERIC_CSUM
> def_bool CPU_LITTLE_ENDIAN

Looks good for powerpc.

Acked-by: Michael Ellerman <[email protected]>

cheers

2014-02-27 01:33:48

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [PATCH v2 1/1] arm64: make a single hook to syscall_trace() for all syscall features

On 02/26/2014 08:25 PM, Will Deacon wrote:
> On Wed, Feb 26, 2014 at 02:00:19AM +0000, AKASHI Takahiro wrote:
>> On 02/26/2014 12:00 AM, Will Deacon wrote:
>>> On Tue, Feb 25, 2014 at 09:14:43AM +0000, AKASHI Takahiro wrote:
>>>> Currently syscall_trace() is called only for ptrace.
>>>> With additional TIF_xx flags defined, it is now called in all the cases
>>>> of audit, ftrace and seccomp in addition to ptrace.
>>>>
>>>> Signed-off-by: AKASHI Takahiro <[email protected]>
>>>> Acked-by: Richard Guy Briggs <[email protected]>
>>>> ---
>>>> arch/arm64/include/asm/thread_info.h | 13 ++++++++++
>>>> arch/arm64/kernel/entry.S | 5 ++--
>>>> arch/arm64/kernel/ptrace.c | 45 +++++++++++++++++-----------------
>>>> 3 files changed, 38 insertions(+), 25 deletions(-)
>>
>> [...]
>>
>>>> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
>>>> index 6a8928b..c70133e 100644
>>>> --- a/arch/arm64/kernel/ptrace.c
>>>> +++ b/arch/arm64/kernel/ptrace.c
>>>> @@ -1062,31 +1062,30 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
>>>> {
>>>> unsigned long saved_reg;
>>>>
>>>> - if (!test_thread_flag(TIF_SYSCALL_TRACE))
>>>> - return regs->syscallno;
>>>> -
>>>> - if (is_compat_task()) {
>>>> - /* AArch32 uses ip (r12) for scratch */
>>>> - saved_reg = regs->regs[12];
>>>> - regs->regs[12] = dir;
>>>> - } else {
>>>> - /*
>>>> - * Save X7. X7 is used to denote syscall entry/exit:
>>>> - * X7 = 0 -> entry, = 1 -> exit
>>>> - */
>>>> - saved_reg = regs->regs[7];
>>>> - regs->regs[7] = dir;
>>>> - }
>>>> + if (test_thread_flag(TIF_SYSCALL_TRACE)) {
>>>> + if (is_compat_task()) {
>>>> + /* AArch32 uses ip (r12) for scratch */
>>>> + saved_reg = regs->regs[12];
>>>> + regs->regs[12] = dir;
>>>> + } else {
>>>> + /*
>>>> + * Save X7. X7 is used to denote syscall entry/exit:
>>>> + * X7 = 0 -> entry, = 1 -> exit
>>>> + */
>>>> + saved_reg = regs->regs[7];
>>>> + regs->regs[7] = dir;
>>>> + }
>>>>
>>>> - if (dir)
>>>> - tracehook_report_syscall_exit(regs, 0);
>>>> - else if (tracehook_report_syscall_entry(regs))
>>>> - regs->syscallno = ~0UL;
>>>> + if (dir)
>>>> + tracehook_report_syscall_exit(regs, 0);
>>>> + else if (tracehook_report_syscall_entry(regs))
>>>> + regs->syscallno = ~0UL;
>>>>
>>>> - if (is_compat_task())
>>>> - regs->regs[12] = saved_reg;
>>>> - else
>>>> - regs->regs[7] = saved_reg;
>>>> + if (is_compat_task())
>>>> + regs->regs[12] = saved_reg;
>>>> + else
>>>> + regs->regs[7] = saved_reg;
>>>> + }
>>>
>>> Aren't these changes (to ptrace.c) just a giant NOP?
>>
>> Umm, the purpose of this big "if" is to run the code only if TIF_SYSCALL_TRACE is set,
>> and to make it easy to add additional hooks, audit and ftrace, around tracehook_report_*()
>> later on.
>
> The existing code already checks TIF_SYSCALL_TRACE. I'd rather you added
> this new code when it's actually nedded (e.g. when adding audit on top).

* This patch is required only if you really merge my audit and/or ftrace patch.
* Putting these changes in audit patch would impose an extra (unnecessary) dependency on ftrace patch.
* Putting them both in audit and ftrace patch would cause a conflict when applying both patches.

Even so, since I don't bother you on this minor issue, I will follow your comment and make changes on:
* arm64: make a single hook to syscall_trace() for all syscall features
* arm64: Add audit support
* arm64: Add ftrace support

-Takahiro AKASHI

> Will
>

2014-02-28 05:16:00

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 0/2] arm64: prerequisites for audit and ftrace

This patchset contains some patches commonly used by audit and ftrace.

Patch [1/2] defines system call related TIF_* flags to add syscall_trace()
hooks, including ftrace, audit and seccomp, later.
Those features will be implemented in separate patchsets, but it's safe to
check for all TIF_* now because they can not be turned on anyway.

Patch [2/2] adds a function which returns a return value of system call.

Changes v1 -> v2:
* added a guard against TIF_SYSCALL_TRACE at tracehook_report_syscall_*()
* renamed _TIF_WORK_SYSCALL to _TIF_SYSCALL_WORK

Changes v2 -> v3:
* reverted a change in syscall_trace() in v1 [1/2]
* added "arm64: Add regs_return_value() in syscall.h" patch which was
previously included in audit patch [2/2]

AKASHI Takahiro (2):
arm64: make a single hook to syscall_trace() for all syscall features
arm64: Add regs_return_value() in syscall.h

arch/arm64/include/asm/ptrace.h | 5 +++++
arch/arm64/include/asm/thread_info.h | 13 +++++++++++++
arch/arm64/kernel/entry.S | 5 +++--
3 files changed, 21 insertions(+), 2 deletions(-)

--
1.7.9.5

2014-02-28 05:16:12

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 1/2] arm64: make a single hook to syscall_trace() for all syscall features

Currently syscall_trace() is called only for ptrace.
With additional TIF_xx flags defined, it is now called in all the cases
of audit, ftrace and seccomp in addition to ptrace.

Acked-by: Richard Guy Briggs <[email protected]>
Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/thread_info.h | 13 +++++++++++++
arch/arm64/kernel/entry.S | 5 +++--
2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index 720e70b..0a8b2a9 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -91,6 +91,9 @@ static inline struct thread_info *current_thread_info(void)
/*
* thread information flags:
* TIF_SYSCALL_TRACE - syscall trace active
+ * TIF_SYSCALL_TRACEPOINT - syscall tracepoint for ftrace
+ * TIF_SYSCALL_AUDIT - syscall auditing
+ * TIF_SECOMP - syscall secure computing
* TIF_SIGPENDING - signal pending
* TIF_NEED_RESCHED - rescheduling necessary
* TIF_NOTIFY_RESUME - callback before returning to user
@@ -101,6 +104,9 @@ static inline struct thread_info *current_thread_info(void)
#define TIF_NEED_RESCHED 1
#define TIF_NOTIFY_RESUME 2 /* callback before returning to user */
#define TIF_SYSCALL_TRACE 8
+#define TIF_SYSCALL_AUDIT 9
+#define TIF_SYSCALL_TRACEPOINT 10
+#define TIF_SECCOMP 11
#define TIF_POLLING_NRFLAG 16
#define TIF_MEMDIE 18 /* is terminating due to OOM killer */
#define TIF_FREEZE 19
@@ -112,10 +118,17 @@ static inline struct thread_info *current_thread_info(void)
#define _TIF_SIGPENDING (1 << TIF_SIGPENDING)
#define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED)
#define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME)
+#define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE)
+#define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT)
+#define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT)
+#define _TIF_SECCOMP (1 << TIF_SECCOMP)
#define _TIF_32BIT (1 << TIF_32BIT)

#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \
_TIF_NOTIFY_RESUME)

+#define _TIF_SYSCALL_WORK (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_AUDIT | \
+ _TIF_SYSCALL_TRACEPOINT | _TIF_SECCOMP)
+
#endif /* __KERNEL__ */
#endif /* __ASM_THREAD_INFO_H */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 0d7b789..6d613cd 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -630,8 +630,9 @@ el0_svc_naked: // compat entry point
enable_irq

get_thread_info tsk
- ldr x16, [tsk, #TI_FLAGS] // check for syscall tracing
- tbnz x16, #TIF_SYSCALL_TRACE, __sys_trace // are we tracing syscalls?
+ ldr x16, [tsk, #TI_FLAGS] // check for syscall hooks
+ tst x16, #_TIF_SYSCALL_WORK
+ b.ne __sys_trace
adr lr, ret_fast_syscall // return address
cmp scno, sc_nr // check upper syscall limit
b.hs ni_sys
--
1.7.9.5

2014-02-28 05:16:23

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v3 2/2] arm64: Add regs_return_value() in syscall.h

This macro, regs_return_value, is used mainly for audit to record system
call's results, but may also be used in test_kprobes.c.

Acked-by Will Deacon <[email protected]>
Acked-by: Richard Guy Briggs <[email protected]>
Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/include/asm/ptrace.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index 0e7fa49..5800ec1 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -134,6 +134,11 @@ struct pt_regs {
#define user_stack_pointer(regs) \
((regs)->sp)

+static inline unsigned long regs_return_value(struct pt_regs *regs)
+{
+ return regs->regs[0];
+}
+
/*
* Are the current registers suitable for user mode? (used to maintain
* security in signal handlers)
--
1.7.9.5

2014-02-28 05:17:46

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v6 0/2] arm64: Add audit support

This patchset adds system call audit support on arm64.
Both 32-bit (AUDIT_ARCH_ARM) and 64-bit tasks (AUDIT_ARCH_AARCH64)
are supported. Since arm64 has the exact same set of system calls
on LE and BE, we don't care about endianness (or more specifically
__AUDIT_ARCH_64BIT bit in AUDIT_ARCH_*).

There are some prerequisites for this patch to work correctly:
* "audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL" patch
* "generic compat system call audit support" patch
* "__NR_* definitions for compat syscalls" patch from Catalin
* "make a single hook to syscall_trace() for all syscall features" patch
* "arm64: Add regs_return_value() in syscall.h" patch
* userspace audit tool (v2.3.2 + my patch for arm64)

Please review them as well for better understandings.

This code was tested on both 32-bit and 64-bit LE userland
in the following two ways:
1) basic operations with auditctl/autrace
# auditctl -a exit,always -S openat -F path=/etc/inittab
# auditctl -a exit,always -F dir=/tmp -F perm=rw
# auditctl -a task,always
# autrace /bin/ls
by comparing output from autrace with one from strace

2) audit-test-code (+ my workarounds for arm/arm64)
by running "audit-tool", "filter" and "syscalls" test categories.

Changes v1 -> v2:
* Modified to utilize "generic compat system call audit" [3/6, 4/6, 5/6]
Please note that a required header, unistd_32.h, is automatically
generated from unistd32.h.
* Refer to regs->orig_x0 instead of regs->x0 as the first argument of
system call in audit_syscall_entry() [6/6]
* Include "Add regs_return_value() in syscall.h" patch [2/6],
which was not intentionally included in v1 because it could be added
by "kprobes support".

Changes v2 -> v3:
* Remove asm/audit.h.
See "generic compat syscall audit support" patch v4
* Remove endianness dependency, ie. AUDIT_ARCH_ARMEB/AARCH64EB.
* Remove kernel/syscalls/Makefile which was used to create unistd32.h.
See Catalin's "Add __NR_* definitions for compat syscalls" patch

Changes v3 -> v4:
* Modified to sync with the patch, "make a single hook to syscall_trace()
for all syscall features"
* aligned with "audit: Add CONFIG_HAVE_ARCH_AUDITSYSCALL" patch

Changes v4 -> v5:
* rebased to 3.14-rcX
* added a guard against TIF_SYSCALL_AUDIT [3/3]
* aligned with the change in "arm64: make a single hook to syscall_trace()
for all syscall features" v2 [3/3]

Changes v5 -> v6:
* removed and put "arm64: Add regs_return_value() in syscall.h" patch into
a separate set
* aligned with the change in "arm64: make a single hook to syscall_trace()
for all syscall features" v3 [1/2]

AKASHI Takahiro (2):
arm64: Add audit support
arm64: audit: Add audit hook in ptrace/syscall_trace

arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/syscall.h | 15 +++++++++++
arch/arm64/kernel/ptrace.c | 54 ++++++++++++++++++++++----------------
include/uapi/linux/audit.h | 1 +
4 files changed, 49 insertions(+), 22 deletions(-)

--
1.7.9.5

2014-02-28 05:18:08

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v6 1/2] arm64: Add audit support

On AArch64, audit is supported through generic lib/audit.c and
compat_audit.c, and so this patch adds arch specific definitions required.

Acked-by Will Deacon <[email protected]>
Acked-by: Richard Guy Briggs <[email protected]>
Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/syscall.h | 15 +++++++++++++++
include/uapi/linux/audit.h | 1 +
3 files changed, 17 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 27bbcfc..aa47548 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -25,6 +25,7 @@ config ARM64
select GENERIC_STRNLEN_USER
select GENERIC_TIME_VSYSCALL
select HARDIRQS_SW_RESEND
+ select HAVE_ARCH_AUDITSYSCALL
select HAVE_ARCH_JUMP_LABEL
select HAVE_ARCH_TRACEHOOK
select HAVE_DEBUG_BUGVERBOSE
diff --git a/arch/arm64/include/asm/syscall.h b/arch/arm64/include/asm/syscall.h
index 70ba9d4..6900183 100644
--- a/arch/arm64/include/asm/syscall.h
+++ b/arch/arm64/include/asm/syscall.h
@@ -16,7 +16,9 @@
#ifndef __ASM_SYSCALL_H
#define __ASM_SYSCALL_H

+#include <linux/audit.h>
#include <linux/err.h>
+#include <asm/compat.h>


static inline int syscall_get_nr(struct task_struct *task,
@@ -104,4 +106,17 @@ static inline void syscall_set_arguments(struct task_struct *task,
memcpy(&regs->regs[i], args, n * sizeof(args[0]));
}

+/*
+ * We don't care about endianness (__AUDIT_ARCH_LE bit) here because
+ * AArch64 has the same system calls both on little- and big- endian.
+ */
+static inline int syscall_get_arch(struct task_struct *task,
+ struct pt_regs *regs)
+{
+ if (is_compat_thread(task_thread_info(task)))
+ return AUDIT_ARCH_ARM;
+
+ return AUDIT_ARCH_AARCH64;
+}
+
#endif /* __ASM_SYSCALL_H */
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 624df43..aa86fab 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -333,6 +333,7 @@ enum {
/* distinguish syscall tables */
#define __AUDIT_ARCH_64BIT 0x80000000
#define __AUDIT_ARCH_LE 0x40000000
+#define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ARMEB (EM_ARM)
--
1.7.9.5

2014-02-28 05:18:19

by AKASHI Takahiro

[permalink] [raw]
Subject: [PATCH v6 2/2] arm64: audit: Add audit hook in ptrace/syscall_trace

This patch adds auditing functions on entry to or exit from
every system call invocation.

Acked-by: Richard Guy Briggs <[email protected]>
Signed-off-by: AKASHI Takahiro <[email protected]>
---
arch/arm64/kernel/ptrace.c | 54 ++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 22 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6a8928b..d4ce70e 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -19,6 +19,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

+#include <linux/audit.h>
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/mm.h>
@@ -38,6 +39,7 @@
#include <asm/compat.h>
#include <asm/debug-monitors.h>
#include <asm/pgtable.h>
+#include <asm/syscall.h>
#include <asm/traps.h>
#include <asm/system_misc.h>

@@ -1062,31 +1064,39 @@ asmlinkage int syscall_trace(int dir, struct pt_regs *regs)
{
unsigned long saved_reg;

- if (!test_thread_flag(TIF_SYSCALL_TRACE))
- return regs->syscallno;
+ if (dir && test_thread_flag(TIF_SYSCALL_AUDIT))
+ audit_syscall_exit(regs);
+
+ if (test_thread_flag(TIF_SYSCALL_TRACE)) {
+ if (is_compat_task()) {
+ /* AArch32 uses ip (r12) for scratch */
+ saved_reg = regs->regs[12];
+ regs->regs[12] = dir;
+ } else {
+ /*
+ * Save X7. X7 is used to denote syscall entry/exit:
+ * X7 = 0 -> entry, = 1 -> exit
+ */
+ saved_reg = regs->regs[7];
+ regs->regs[7] = dir;
+ }

- if (is_compat_task()) {
- /* AArch32 uses ip (r12) for scratch */
- saved_reg = regs->regs[12];
- regs->regs[12] = dir;
- } else {
- /*
- * Save X7. X7 is used to denote syscall entry/exit:
- * X7 = 0 -> entry, = 1 -> exit
- */
- saved_reg = regs->regs[7];
- regs->regs[7] = dir;
- }
+ if (dir)
+ tracehook_report_syscall_exit(regs, 0);
+ else if (tracehook_report_syscall_entry(regs))
+ regs->syscallno = ~0UL;

- if (dir)
- tracehook_report_syscall_exit(regs, 0);
- else if (tracehook_report_syscall_entry(regs))
- regs->syscallno = ~0UL;
+ if (is_compat_task())
+ regs->regs[12] = saved_reg;
+ else
+ regs->regs[7] = saved_reg;
+ }

- if (is_compat_task())
- regs->regs[12] = saved_reg;
- else
- regs->regs[7] = saved_reg;
+ if (!dir && test_thread_flag(TIF_SYSCALL_AUDIT))
+ audit_syscall_entry(syscall_get_arch(current, regs),
+ (int)regs->syscallno,
+ regs->orig_x0, regs->regs[1],
+ regs->regs[2], regs->regs[3]);

return regs->syscallno;
}
--
1.7.9.5

2014-02-28 15:58:03

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v3 1/2] arm64: make a single hook to syscall_trace() for all syscall features

On Fri, Feb 28, 2014 at 05:14:24AM +0000, AKASHI Takahiro wrote:
> Currently syscall_trace() is called only for ptrace.
> With additional TIF_xx flags defined, it is now called in all the cases
> of audit, ftrace and seccomp in addition to ptrace.
>
> Acked-by: Richard Guy Briggs <[email protected]>
> Signed-off-by: AKASHI Takahiro <[email protected]>

Acked-by: Will Deacon <[email protected]>

Will

2014-02-28 16:16:37

by Will Deacon

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] arm64: audit: Add audit hook in ptrace/syscall_trace

On Fri, Feb 28, 2014 at 05:17:15AM +0000, AKASHI Takahiro wrote:
> This patch adds auditing functions on entry to or exit from
> every system call invocation.
>
> Acked-by: Richard Guy Briggs <[email protected]>
> Signed-off-by: AKASHI Takahiro <[email protected]>
> ---
> arch/arm64/kernel/ptrace.c | 54 ++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 22 deletions(-)

I think you need to do something like I did for arch/arm/, where we have
separate trace functions for entry/exit to make sure that we invoke the
various helpers in the correct order (for example, you want to invoke all
the debug stuff *first* on entry, but *last* on exit).

Will

2014-02-28 20:46:43

by Richard Guy Briggs

[permalink] [raw]
Subject: Re: [PATCH v6 2/2] arm64: audit: Add audit hook in ptrace/syscall_trace

On 14/02/28, Will Deacon wrote:
> On Fri, Feb 28, 2014 at 05:17:15AM +0000, AKASHI Takahiro wrote:
> > This patch adds auditing functions on entry to or exit from
> > every system call invocation.
> >
> > Acked-by: Richard Guy Briggs <[email protected]>
> > Signed-off-by: AKASHI Takahiro <[email protected]>
> > ---
> > arch/arm64/kernel/ptrace.c | 54 ++++++++++++++++++++++++++------------------
> > 1 file changed, 32 insertions(+), 22 deletions(-)
>
> I think you need to do something like I did for arch/arm/, where we have
> separate trace functions for entry/exit to make sure that we invoke the
> various helpers in the correct order (for example, you want to invoke all
> the debug stuff *first* on entry, but *last* on exit).

I'd have to agree. I've just had my head deep in audit_syscall_entry()
and syscall_get_arch to clean them up. Since current is only ever fed
to syscall_get_arch() and regs is never used by syscall_get_arch(), I'm
looking at dropping both from the syscall_get_arch() args list, but
leave syscall_get_arch() as you have it for now.

> Will

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545