diff mbox

[v6,2/2] arm64: audit: Add audit hook in ptrace/syscall_trace

Message ID 1393564635-3921-3-git-send-email-takahiro.akashi@linaro.org
State New
Headers show

Commit Message

AKASHI Takahiro Feb. 28, 2014, 5:17 a.m. UTC
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(-)

Comments

Will Deacon Feb. 28, 2014, 4:15 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Richard Guy Briggs Feb. 28, 2014, 8:45 p.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
AKASHI Takahiro March 6, 2014, 2:10 a.m. UTC | #3
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Richard Guy Briggs March 6, 2014, 2:55 a.m. UTC | #4
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

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;
 }