Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9cc1d42ab0495ff3f0113709966b47e8dab41266.camel@kernel.org>
Subject: Re: [PATCH v2 0/3] tracing: Fix synthetic event bug
From:   Tom Zanussi <zanussi@kernel.org>
To:     Steven Rostedt <rostedt@goodmis.org>, linux-kernel@vger.kernel.org
Cc:     Masami Hiramatsu <mhiramat@kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>
Date:   Wed, 12 Oct 2022 10:06:21 -0500
In-Reply-To: <20221012104055.421393330@goodmis.org>
References: <20221012104055.421393330@goodmis.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.1-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

Hi Steve,

On Wed, 2022-10-12 at 06:40 -0400, Steven Rostedt wrote:
>=20
> The follow commands caused a crash:
>=20
> =C2=A0 # cd /sys/kernel/tracing
> =C2=A0 # echo 's:open char file[]' > dynamic_events
> =C2=A0 # echo
> 'hist:keys=3Dcommon_pid:file=3Dfilename:onchange($file).trace(open,$file)
> ' > events/syscalls/sys_enter_openat/trigger'
> =C2=A0 # echo 1 > events/synthetic/open/enable
>=20
> BOOM!
>=20
> The problem is that the synthetic event field "char file[]" will read
> the value given to it as a string without any memory checks to make
> sure
> the address is valid. The above example will pass in the user space
> address and the sythetic event code will happily call strlen() on it
> and then strscpy() where either one will cause an oops when accessing
> user space addresses.
>=20
> Changes since v1:
> https://lore.kernel.org/all/20221011212501.773319898@goodmis.org/
>=20
> =C2=A0- Handle "(fault)" printing when there's a fault


Thanks for fixing the synthetic event string tracing bug, along with
the other nice cleanup.

Reviewed-by: Tom Zanussi <zanussi@kernel.org>


>=20
> Steven Rostedt (Google) (3):
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tracing: Move duplicate code of trace_kpro=
be/eprobe.c into
> header
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tracing: Add "(fault)" name injection to k=
ernel probes
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 tracing: Fix reading strings from syntheti=
c events
>=20
> ----
> =C2=A0kernel/trace/trace_eprobe.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=
=C2=A0 60 ++------------------
> =C2=A0kernel/trace/trace_events_synth.c |=C2=A0 23 ++++++--
> =C2=A0kernel/trace/trace_kprobe.c=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=
=C2=A0 60 ++------------------
> =C2=A0kernel/trace/trace_probe_kernel.h | 115
> ++++++++++++++++++++++++++++++++++++++
> =C2=A04 files changed, 142 insertions(+), 116 deletions(-)
> =C2=A0create mode 100644 kernel/trace/trace_probe_kernel.h