Re: [RFC PATCH v2 1/1] audit: Add generic compat syscall support

On 11/26/2013 04:01 AM, Will Deacon wrote: >On Tue, Nov 19, 2013 at 09:43:55AM +0000, AKASHI Takahiro wrote: >>(v1 was created mistakenly. Please igore it.) >> >>lib/audit.c provides a generic definition for auditing system calls. >>lib/compat_audit.c similarly adds compat syscall support for >>bi-architectures (32/64-bit). >> >>Each architecture must define audit_is_compat() in asm/audit.h. >> >>Signed-off-by: AKASHI Takahiro <takahiro.akashi(a)linaro.org&gt; >>--- >> include/linux/audit.h | 9 +++++++++ >> lib/Makefile | 3 +++ >> lib/audit.c | 17 +++++++++++++++++ >> lib/compat_audit.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 80 insertions(+) >> create mode 100644 lib/compat_audit.c >> >>diff --git a/include/linux/audit.h b/include/linux/audit.h >>index 729a4d1..c49a312 100644 >>--- a/include/linux/audit.h >>+++ b/include/linux/audit.h >>@@ -76,6 +76,15 @@ struct audit_field { >> extern int __init audit_register_class(int class, unsigned *list); >> extern int audit_classify_syscall(int abi, unsigned syscall); >> extern int audit_classify_arch(int arch); >>+#if defined(CONFIG_AUDIT_GENERIC) && defined(CONFIG_COMPAT) >>+extern unsigned compat_write_class[]; >>+extern unsigned compat_read_class[]; >>+extern unsigned compat_dir_class[]; >>+extern unsigned compat_chattr_class[]; >>+extern unsigned compat_signal_class[]; >>+ >>+extern int audit_classify_compat_syscall(int abi, unsigned syscall); >>+#endif >> >> /* audit_names->type values */ >> #define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */ >>diff --git a/lib/Makefile b/lib/Makefile >>index f3bb2cb..5bb185a 100644 >>--- a/lib/Makefile >>+++ b/lib/Makefile >>@@ -96,6 +96,9 @@ obj-$(CONFIG_TEXTSEARCH_BM) += ts_bm.o >> obj-$(CONFIG_TEXTSEARCH_FSM) += ts_fsm.o >> obj-$(CONFIG_SMP) += percpu_counter.o >> obj-$(CONFIG_AUDIT_GENERIC) += audit.o >>+ifeq ($(CONFIG_COMPAT),y) >>+obj-$(CONFIG_AUDIT_GENERIC) += compat_audit.o >>+endif >> >> obj-$(CONFIG_SWIOTLB) += swiotlb.o >> obj-$(CONFIG_IOMMU_HELPER) += iommu-helper.o >>diff --git a/lib/audit.c b/lib/audit.c >>index 76bbed4..3bf3858 100644 >>--- a/lib/audit.c >>+++ b/lib/audit.c >>@@ -1,6 +1,7 @@ >> #include <linux/init.h> >> #include <linux/types.h> >> #include <linux/audit.h> >>+#include <asm/audit.h> >> #include <asm/unistd.h> >> >> static unsigned dir_class[] = { >>@@ -30,11 +31,20 @@ static unsigned signal_class[] = { >> >> int audit_classify_arch(int arch) >> { >>+#ifdef CONFIG_COMPAT >>+ if (audit_is_compat(arch)) >>+ return 1; >>+#endif >> return 0; >> } >> >> int audit_classify_syscall(int abi, unsigned syscall) >> { >>+#ifdef CONFIG_COMPAT >>+ if (audit_is_compat(abi)) >>+ return audit_classify_compat_syscall(abi, syscall); >>+#endif > >Hmm, I'm not sure this is the right way to solve this problem. Whether >something is compat or not depends on the task to which it is associated. If >this is always the current task for the audit cases, then you can just use >something like is_compat_task. Otherwise, I think we need to get a handle on >the task_struct here. An arch-callback feels like the wrong approach to me. You are completely right. In my current (v3 prototype) implementation, "abi" argument, which can be AUDIT_ARCH_ARM(EB) or AUDIT_ARCH_AARCH64(EB), passed to audit_classify_syscall() is determined per-task using is_compat_thread() when audit_syscall_entry() is executed in syscall_trace(). (Obviously audit_is_compat() is true only in case of AUDIT_ARCH_ARM.) V3 based on this patch is working for 32-bit and 64-bit userland. I can submit v3 patch if you want.

Thanks, -Takahiro AKASHI >Will