linux-kernel - Re: [PATCH v3 3/5] tracing: Update synth command errors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201208125340.407150f2@gandalf.local.home>
Date:   Tue, 8 Dec 2020 12:53:40 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Tom Zanussi <zanussi@...nel.org>
Cc:     axelrasmussen@...gle.com, mhiramat@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/5] tracing: Update synth command errors

On Tue, 08 Dec 2020 11:34:41 -0600
Tom Zanussi <zanussi@...nel.org> wrote:

> Unfortunately, you're correct, if you have a script that creates a
> synthetic event without semicolons, this patchset will break it, as I
> myself found out and fixed in patch 4 ([PATCH v3 4/5] selftests/ftrace:
> Add synthetic event field separators) [4].
> 
> So whereas before this would work, even though it shouldn't have in the
> first place:
> 
>   # echo 'wakeup_latency  u64 lat pid_t pid char comm[16]' >
> synthetic_events
> 
> it now has to be:
> 
>   # echo 'wakeup_latency  u64 lat; pid_t pid; char comm[16]' >
> synthetic_events
> 
> So yeah, this patchset fixes a set of parsing bugs for things that
> shouldn't have been accepted as valid, but shouldn't break things that
> are obviously valid.
> 
> If it's too late to fix them, though, I guess we'll just have to live
> with them, or some other option?

I would suggest allowing the old interface work (with no new features, for
backward compatibility), but new things like "char comm[16]" we require
semicolons.

One method to do this is to add to the start of reading the string, and
checking if it has semicolons. If it does not, we create a new string with
them, but make sure that the string does not include new changes.

	strncpy_from_user(buffer, user_buff, sizeof(buffer));

	if (!strstr(buffer, ";")) {
		if (!audit_old_buffer(buffer))
			goto error;
		insert_colons(buffer);
	}

That is, if the buffer does not have semicolons, then check if it is a
valid "old format", and if not, we error out. Otherwise, we insert the
colons into the buffer, and process that as if the user put in colons:

That is:

	echo 'wakeup_latency u64 lat pid_t pid' > synthetic_events

would change the buffer to:

	"wakeup_latency u64 lat; pid_t pid;"

And then put it through the normal processing. I think its OK that if the
user were to cat out the synthetic events, it would see the semicolons even
if it did not add them. As I don't think that will break userspace.

Does that make sense?

-- Steve