[PATCH V2 6/6] arm64: Add uprobe support

Hi Catalin,

On Sun, Oct 30, 2016 at 7:39 PM, Catalin Marinas
[off-list ref] wrote:

On Tue, Sep 27, 2016 at 01:18:00PM +0530, Pratyush Anand wrote:

quoted

--- /dev/null
+++ b/arch/arm64/kernel/probes/uprobes.c

@@ -0,0 +1,221 @@
+/*
+ * Copyright (C) 2014-2016 Pratyush Anand <panand@redhat.com>
+ *
+ * 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.
+ */
+#include <linux/highmem.h>
+#include <linux/ptrace.h>
+#include <linux/uprobes.h>
+#include <asm/cacheflush.h>
+
+#include "decode-insn.h"
+
+#define UPROBE_INV_FAULT_CODE        UINT_MAX
+
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+     return false;
+}

On the previous series, I had a comment left unanswered with regards to
always returning false in is_trap_insn():

Looking at handle_swbp(), if we hit a breakpoint for which we don't have
a valid uprobe, this function currently sends a SIGTRAP. But if
is_trap_insn() returns false always, is_trap_at_addr() would return 0 in
this case so the SIGTRAP is never issued.

A agreed on this that the older implementation i.e. the default one of
is_trap_insn() is better for the time being.

probably 'strtle r0, [r0], #160' would have the closest matching
aarch32 instruction wrt BRK64_OPCODE_UPROBES(0xd42000A0). But that too
seems a bad instruction. So, there might not be any aarch32
instruction which will match to uprobe BRK instruction.


Therefore, if I send a V3 by removing aacrh64
Hi Catalin,

On Sun, Oct 30, 2016 at 7:39 PM, Catalin Marinas
[off-list ref] wrote:

On Tue, Sep 27, 2016 at 01:18:00PM +0530, Pratyush Anand wrote:

quoted

--- /dev/null
+++ b/arch/arm64/kernel/probes/uprobes.c

@@ -0,0 +1,221 @@
+/*
+ * Copyright (C) 2014-2016 Pratyush Anand <panand@redhat.com>
+ *
+ * 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.
+ */
+#include <linux/highmem.h>
+#include <linux/ptrace.h>
+#include <linux/uprobes.h>
+#include <asm/cacheflush.h>
+
+#include "decode-insn.h"
+
+#define UPROBE_INV_FAULT_CODE        UINT_MAX
+
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+     return false;
+}

On the previous series, I had a comment left unanswered with regards to
always returning false in is_trap_insn():

Looking at handle_swbp(), if we hit a breakpoint for which we don't have
a valid uprobe, this function currently sends a SIGTRAP. But if
is_trap_insn() returns false always, is_trap_at_addr() would return 0 in
this case so the SIGTRAP is never issued.

A agreed on this that the older implementation i.e. the default one of
is_trap_insn() is better for the time being.

probably 'strtle r0, [r0], #160' would have the closest matching
aarch32 instruction wrt BRK64_OPCODE_UPROBES(0xd42000A0). But that too
seems a bad instruction. So, there might not be any aarch32
instruction which will match to uprobe BRK instruction.


Therefore, if I send a V3 by removing aacrh64
Hi Catalin,

On Sun, Oct 30, 2016 at 7:39 PM, Catalin Marinas
[off-list ref] wrote:

On Tue, Sep 27, 2016 at 01:18:00PM +0530, Pratyush Anand wrote:

quoted

--- /dev/null
+++ b/arch/arm64/kernel/probes/uprobes.c

@@ -0,0 +1,221 @@
+/*
+ * Copyright (C) 2014-2016 Pratyush Anand <panand@redhat.com>
+ *
+ * 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.
+ */
+#include <linux/highmem.h>
+#include <linux/ptrace.h>
+#include <linux/uprobes.h>
+#include <asm/cacheflush.h>
+
+#include "decode-insn.h"
+
+#define UPROBE_INV_FAULT_CODE        UINT_MAX
+
+bool is_trap_insn(uprobe_opcode_t *insn)
+{
+     return false;
+}

On the previous series, I had a comment left unanswered with regards to
always returning false in is_trap_insn():

Looking at handle_swbp(), if we hit a breakpoint for which we don't have
a valid uprobe, this function currently sends a SIGTRAP. But if
is_trap_insn() returns false always, is_trap_at_addr() would return 0 in
this case so the SIGTRAP is never issued.

A agreed on this that the older implementation i.e. the default one of
is_trap_insn() is better for the time being. I sent out V2 before your
last comment on it in V1 :(.

probably 'strtle r0, [r0], #160' would have the closest matching
aarch32 instruction wrt BRK64_OPCODE_UPROBES(0xd42000A0). But that too
seems a bad aarch32 instruction. So, there might not be any aarch32
instruction which will match to uprobe BRK instruction.


Therefore, if I send a V3 by removing aacrh64 is_trap_insn(), would
that be acceptable, or do you see any other issue with this patch
series? If there is anything else, I would address that in V3 as well.

Thanks for your review.

~Pratyush