linux-kernel - Re: [PATCH v3 09/19] unwind: Introduce sframe user space unwinding

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: <20241031230313.ubybve4r7mlbcbuu@jpoimboe>
Date: Thu, 31 Oct 2024 16:03:13 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: x86@...nel.org, Peter Zijlstra <peterz@...radead.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Ingo Molnar <mingo@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	linux-kernel@...r.kernel.org, Indu Bhagat <indu.bhagat@...cle.com>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
	Ian Rogers <irogers@...gle.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	linux-perf-users@...r.kernel.org, Mark Brown <broonie@...nel.org>,
	linux-toolchains@...r.kernel.org, Jordan Rome <jordalgo@...a.com>,
	Sam James <sam@...too.org>, linux-trace-kernel@...r.kerne.org,
	Jens Remus <jremus@...ux.ibm.com>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Florian Weimer <fweimer@...hat.com>,
	Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCH v3 09/19] unwind: Introduce sframe user space unwinding

On Thu, Oct 31, 2024 at 01:57:10PM -0700, Andrii Nakryiko wrote:
> > > what if ip_off is larger than 4GB? ELF section can be bigger than 4GB, right?
> >
> > That's baked into sframe v2.
> 
> I believe we do have large production binaries with more than 4GB of
> text, what are we going to do about them? It would be interesting to
> hear sframe people's opinion. Adding such a far-reaching new format in
> 2024 with these limitations is kind of sad. At the very least maybe we
> should allow some form of chaining sframe definitions to cover more
> than 4GB segments? Please CC relevant folks, I'm wondering what
> they're thinking about this.

Personally I find the idea of a single 4GB+ text segment pretty
surprising as I've never seen anything even close to that.

Anyway it's iterative development and not everybody's requirements are
clear from day 1.  Which is why we're discussing it now.  I think there
are already plans to do an sframe v3.

> > > > +                       if (text_vma) {
> > > > +                               pr_warn_once("%s[%d]: multiple EXEC segments unsupported\n",
> > > > +                                            current->comm, current->pid);
> > >
> > > is this just something that fundamentally can't be supported by SFrame
> > > format? Or just an implementation simplification?
> >
> > It's a simplification I suppose.
> 
> That's a rather random limitation, IMO... How hard would it be to not
> make that assumption?

It's definitely not random, there's no need to complicate the code if
this condition doesn't exist.

> > > It's not illegal to have an executable with multiple VM_EXEC segments,
> > > no? Should this be a pr_warn_once() then?
> >
> > I don't know, is it allowed?  I've never seen it in practice.  The
> 
> I'm pretty sure you can do that with a custom linker script, at the
> very least. Normally this probably won't happen, but I don't think
> Linux dictates how many executable VMAs an application can have.
> And it probably just naturally happens for JIT-ted applications (Java,
> Go, etc).

Actually I just double checked and even the kernel's ELF loader assumes
that each executable has only a single text start+end address pair.

> > pr_warn_once() is not reporting that it's illegal but rather that this
> > corner case actually exists and maybe needs to be looked at.
> 
> This warn() will be logged across millions of machines in the fleet,
> triggering alarms, people looking at this, making custom internal
> patches to disable the known-to-happen warn. Why do we need all this?
> This is an issue that is trivial to trigger by user process that's not
> doing anything illegal. Why?

There's no point in adding complexity to support some hypothetical.  I
can remove the printk though.

-- 
Josh