From mboxrd@z Thu Jan  1 00:00:00 1970
From: Aleksa Sarai <cyphar@cyphar.com>
Subject: Re: [PATCH v3 1/2] kretprobe: produce sane stack traces
Date: Fri, 2 Nov 2018 08:13:43 +1100
Message-ID: <20181101211343.yooxwqfnoloprb5h@yavin>
References: <20181101083551.3805-1-cyphar@cyphar.com>
 <20181101083551.3805-2-cyphar@cyphar.com>
 <20181102002039.8f22c10fa47cae75fa709165@kernel.org>
Mime-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="ywyssjdmzzy34rtq"
Cc: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jonathan Corbet <corbet@lwn.net>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Arnaldo Carvalho de Melo <acme@kernel.org>,
        Alexander Shishkin <alexander.shishkin@linux.intel.com>,
        Jiri Olsa <jolsa@redhat.com>,
        Namhyung Kim <namhyung@kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Shuah Khan <shuah@kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Brendan Gregg <bgregg@netflix.com>,
        Christian Brauner <christian@brauner.io>,
        Aleksa Sarai <asarai@suse.de>, netdev@vger.kernel.org,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
To: Masami Hiramatsu <mhiramat@kernel.org>
Return-path: <linux-kernel-owner@vger.kernel.org>
Content-Disposition: inline
In-Reply-To: <20181102002039.8f22c10fa47cae75fa709165@kernel.org>
Sender: linux-kernel-owner@vger.kernel.org
List-Id: netdev.vger.kernel.org


--ywyssjdmzzy34rtq
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On 2018-11-02, Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Please split the test case as an independent patch.

Will do. Should the Documentation/ change also be a separate patch?

> > new file mode 100644
> > index 000000000000..03146c6a1a3c
> > --- /dev/null
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kretprobe_stacktrace=
=2Etc
> > @@ -0,0 +1,25 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# description: Kretprobe dynamic event with a stacktrace
> > +
> > +[ -f kprobe_events ] || exit_unsupported # this is configurable
> > +
> > +echo 0 > events/enable
> > +echo 1 > options/stacktrace
> > +
> > +echo 'r:teststackprobe sched_fork $retval' > kprobe_events
> > +grep teststackprobe kprobe_events
> > +test -d events/kprobes/teststackprobe
>=20
> Hmm, what happen if we have 2 or more kretprobes on same stack?
> It seems you just save stack in pre_handler, but that stack can already
> includes another kretprobe's trampline address...

Yeah, you're quite right...

My first instinct was to do something awful like iterate over the set of
"kretprobe_instance"s with ->task =3D=3D current, and then correct
kretprobe_trampoline entries using ->ret_addr. (I think this would be
correct because each task can only be in one context at once, and in
order to get to a particular kretprobe all of your caller kretprobes
were inserted before you and any sibling or later kretprobe_instances
will have been removed. But I might be insanely wrong on this front.)

However (as I noted in the other thread), there is a problem where
kretprobe_trampoline actually stops the unwinder in its tracks and thus
you only get the first kretprobe_trampoline. This is something I'm going
to look into some more (despite not having made progress on it last
time) since now it's something that actually needs to be fixed (and
as I mentioned in the other thread, show_stack() actually works on x86
in this context unlike the other stack_trace users).

--=20
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

--ywyssjdmzzy34rtq
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEb6Gz4/mhjNy+aiz1Snvnv3Dem58FAlvbbIcACgkQSnvnv3De
m5+H3g//U3dUHafkMVldC9z5E7aA1ljh+ZbM/PA/HhWq6BaNUkibEKu4/M4TYi6g
FGphK06koWBePb0HkeqEQ7gspO1FPxwRUwn/DJNAM3yR+VLIZftkwwjQadFpKMVX
T3TTYFf4ogUMeqQPRBld3cVK23ILeiokik02tRcN2Tq1dP3vish0TgH4Z38Rbiql
LSbFhRz62GJ99yXrUlgoircGDFKCrCCJBTd7JuN4XXZk4PHJfpsmp9aQQMWY5u2Z
IjgNQt4nuArS7eloy8WtlyYNsOhGwf7eyqEsAc1jwcYCMRq1C2zth6vDg3cIk7+K
G1s3ZX40jE8a7/w7H09dp0PLgHHuXWZChKdyvTzWmXyEiKKjm+wNBF5dGiELPoqX
+K94Mp9LLbOFOCg7yCZDHaal67xktu9o5G8zmpaVNSFfrgLSSiXXh6mNcqSJ4psI
CdRSU51NlRtf9cXhsHKZ8s2FMW1bH2q6GVR9cEazCIDdLHDrbDD6PfvpK7NBPiru
NlZ9U4GKrm2e/SQJrfMOzukI2ftqmjJzRvD/bytNvDjVn3k2hu3gb+1XFm2VzZaK
FaOJFXhQWr0oP+yT4hoTAiPWquXcYA4trlhy88WqKkNMPzB0hhHSr9VVediVfyYH
AnBW1Q8Uv9GaKyBe5LIkaMth3u36Cf5ustLvjfweVYkHak9Obaw=
=BZZ/
-----END PGP SIGNATURE-----

--ywyssjdmzzy34rtq--