[PATCH AUTOSEL for 4.9 005/219] kretprobes: Ensure probe location is at function entry

Subsystems: kprobes, the rest

STALE3057d REVIEWED: 9 (9M)

1 review trailer (1 from subsystem maintainers).

3 messages, 2 authors, 2018-03-05 · open the first message on its own page

[PATCH AUTOSEL for 4.9 005/219] kretprobes: Ensure probe location is at function entry

From: Sasha Levin <hidden>
Date: 2018-03-03 22:31:05

From: "Naveen N. Rao" <redacted>

[ Upstream commit 90ec5e89e393c76e19afc845d8f88a5dc8315919 ]

kretprobes can be registered by specifying an absolute address or by
specifying offset to a symbol. However, we need to ensure this falls at
function entry so as to be able to determine the return address.

Validate the same during kretprobe registration. By default, there
should not be any offset from a function entry, as determined through a
kallsyms_lookup(). Introduce arch_function_offset_within_entry() as a
way for architectures to override this.

Signed-off-by: Naveen N. Rao <redacted>
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Ananth N Mavinakayanahalli <redacted>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: linuxppc-dev@lists.ozlabs.org
Link: http://lkml.kernel.org/r/f1583bc4839a3862cfc2acefcc56f9c8837fa2ba.148=
7770934.git.naveen.n.rao@linux.vnet.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <redacted>
Signed-off-by: Sasha Levin <redacted>
---
 include/linux/kprobes.h |  1 +
 kernel/kprobes.c        | 13 +++++++++++++
 2 files changed, 14 insertions(+)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index e23392517db9..56a998f34bdd 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -266,6 +266,7 @@ extern int arch_init_kprobes(void);
 extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
+extern bool arch_function_offset_within_entry(unsigned long offset);
=20
 extern bool within_kprobe_blacklist(unsigned long addr);
=20
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index a1a07cf1101f..2ede39e4227f 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1834,12 +1834,25 @@ static int pre_handler_kretprobe(struct kprobe *p, =
struct pt_regs *regs)
 }
 NOKPROBE_SYMBOL(pre_handler_kretprobe);
=20
+bool __weak arch_function_offset_within_entry(unsigned long offset)
+{
+	return !offset;
+}
+
 int register_kretprobe(struct kretprobe *rp)
 {
 	int ret =3D 0;
 	struct kretprobe_instance *inst;
 	int i;
 	void *addr;
+	unsigned long offset;
+
+	addr =3D kprobe_addr(&rp->kp);
+	if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, &offset))
+		return -EINVAL;
+
+	if (!arch_function_offset_within_entry(offset))
+		return -EINVAL;
=20
 	if (kretprobe_blacklist_size) {
 		addr =3D kprobe_addr(&rp->kp);
--=20
2.14.1

Re: [PATCH AUTOSEL for 4.9 005/219] kretprobes: Ensure probe location is at function entry

From: Naveen N. Rao <hidden>
Date: 2018-03-05 07:03:09

Hi Sasha,

Sasha Levin wrote:
From: "Naveen N. Rao" <redacted>
=20
[ Upstream commit 90ec5e89e393c76e19afc845d8f88a5dc8315919 ]
=20
Sorry if this is obvious, but why was this patch picked up for -stable? =20
I don't see the upstream commit tagging -stable, so curious why this was=20
done.

I don't think this patch should be pushed to -stable since this is not=20
really a bug fix. There are also other dependencies for this change (see=20
commit a64e3f35a45f4a, for instance), including how userspace (perf)=20
builds out the retprobe argument. As such, please drop this from -stable=20
(for 3.18. 4.4 and 4.9).

Thanks,
Naveen

=

Re: [PATCH AUTOSEL for 4.9 005/219] kretprobes: Ensure probe location is at function entry

From: Sasha Levin <hidden>
Date: 2018-03-05 20:06:43

On Mon, Mar 05, 2018 at 12:32:57PM +0530, Naveen N. Rao wrote:
Hi Sasha,

Sasha Levin wrote:
quoted
From: "Naveen N. Rao" <redacted>

[ Upstream commit 90ec5e89e393c76e19afc845d8f88a5dc8315919 ]
Sorry if this is obvious, but why was this patch picked up for=20
-stable?  I don't see the upstream commit tagging -stable, so curious=20
why this was done.

I don't think this patch should be pushed to -stable since this is not=20
really a bug fix. There are also other dependencies for this change=20
(see commit a64e3f35a45f4a, for instance), including how userspace=20
(perf) builds out the retprobe argument. As such, please drop this=20
from -stable (for 3.18. 4.4 and 4.9).
Hi Naveen,

It's an automatic selection process that attempts to find commits that
should be in stable but weren't tagged as such.

I'll drop this patch, thanks!

--=20

Thanks,
Sasha=
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help