linux-kernel - Re: mmotm 2022-02-11-15-07 uploaded (objtool: ftrace_likely

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] [day] [month] [year] [list]

Message-ID: <20220214101506.3e69ea97@gandalf.local.home>
Date:   Mon, 14 Feb 2022 10:15:06 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     Randy Dunlap <rdunlap@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>, broonie@...nel.org,
        mhocko@...e.cz, sfr@...b.auug.org.au, linux-next@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, mm-commits@...r.kernel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>
Subject: Re: mmotm 2022-02-11-15-07 uploaded (objtool: ftrace_likely_update)

On Sat, 12 Feb 2022 09:06:49 +0100
Peter Zijlstra <peterz@...radead.org> wrote:

> Yes, TRACE_BRANCH_PROFILING and PROFILE_ALL_BRANCHES are fundamentally
> broken and I have no intention of trying to fix them.
> 
> The moment we pull PTI into noinstr C code this will result in insta
> boot fail.

Actually, I don't think anyone has every used the "tracers" for this, and I
will be happy to get rid of it:

void ftrace_likely_update(struct ftrace_likely_data *f, int val,
			  int expect, int is_constant)
{
	unsigned long flags = user_access_save();

	/* A constant is always correct */
	if (is_constant) {
		f->constant++;
		val = expect;
	}


------8<------
	/*
	 * I would love to have a trace point here instead, but the
	 * trace point code is so inundated with unlikely and likely
	 * conditions that the recursive nightmare that exists is too
	 * much to try to get working. At least for now.
	 */
	trace_likely_condition(f, val, expect);
----->8-------

	/* FIXME: Make this atomic! */
	if (val == expect)
		f->data.correct++;
	else
		f->data.incorrect++;

	user_access_restore(flags);
}
EXPORT_SYMBOL(ftrace_likely_update);

The above with the cut lines I added.

I still use the likely and unlikely counters. Would it be possible to mark
that function as "noinstr" and still record them (I don't care if there's
races where we miss a few or add a few too many). But they have been really
affective in finding bad locations of likely and unlikely callers.

As I said. I have no problem with removing the trace portion of that code.
It was more of an academic exercise than a useful one, but the counters
are still very useful to have.

-- Steve