| Message ID | 1393564635-3921-3-git-send-email-takahiro.akashi@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <rgb@redhat.com> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > 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
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 <rgb@redhat.com> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > > --- > > 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 <rbriggs@redhat.com> 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
On 03/01/2014 01:15 AM, 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 <rgb@redhat.com> >> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> >> --- >> 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 > If you mean syscall_trace_enter()/exit(), I will follow your suggestion for readability. -Takahiro AKASHI
On 14/03/06, AKASHI Takahiro wrote: > On 03/01/2014 01:15 AM, 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 <rgb@redhat.com> > >>Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > >>--- > >> 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 > > If you mean syscall_trace_enter()/exit(), I will follow your suggestion > for readability. It isn't so much a question of readability, but rather correctness, undoing operations in the opposite order on exit that they were done on entry. > -Takahiro AKASHI - RGB -- Richard Guy Briggs <rbriggs@redhat.com> 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
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; }