linux-kernel - Re: [PATCH v13 02/14] unwind_user: Add frame pointer support

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: <20250710112147.41585f6a@batman.local.home>
Date: Thu, 10 Jul 2025 11:21:47 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Jens Remus <jremus@...ux.ibm.com>
Cc: Steven Rostedt <rostedt@...nel.org>, linux-kernel@...r.kernel.org,
 linux-trace-kernel@...r.kernel.org, bpf@...r.kernel.org, x86@...nel.org,
 Masami Hiramatsu <mhiramat@...nel.org>, Mathieu Desnoyers
 <mathieu.desnoyers@...icios.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>, Jiri
 Olsa <jolsa@...nel.org>, Namhyung Kim <namhyung@...nel.org>, Thomas
 Gleixner <tglx@...utronix.de>, Andrii Nakryiko <andrii@...nel.org>, Indu
 Bhagat <indu.bhagat@...cle.com>, "Jose E. Marchesi" <jemarch@....org>, Beau
 Belgrave <beaub@...ux.microsoft.com>, Linus Torvalds
 <torvalds@...ux-foundation.org>, Andrew Morton <akpm@...ux-foundation.org>,
 Jens Axboe <axboe@...nel.dk>, Florian Weimer <fweimer@...hat.com>, Sam
 James <sam@...too.org>, Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik
 <gor@...ux.ibm.com>
Subject: Re: [PATCH v13 02/14] unwind_user: Add frame pointer support

On Wed, 9 Jul 2025 12:01:14 +0200
Jens Remus <jremus@...ux.ibm.com> wrote:
> >  static int unwind_user_next(struct unwind_user_state *state)
> >  {
> > -	/* no implementation yet */
> > +	struct unwind_user_frame *frame;
> > +	unsigned long cfa = 0, fp, ra = 0;
> > +	unsigned int shift;
> > +
> > +	if (state->done)
> > +		return -EINVAL;
> > +
> > +	if (fp_state(state))
> > +		frame = &fp_frame;
> > +	else
> > +		goto done;
> > +
> > +	if (frame->use_fp) {
> > +		if (state->fp < state->sp)  
> 
> 		if (state->fp <= state->sp)
> 
> I meanwhile came to the conclusion that for architectures, such as s390,
> where SP at function entry == SP at call site, the FP may be equal to
> the SP.  At least for the brief period where the FP has been setup and
> stack allocation did not yet take place.  For most architectures this
> can probably only occur in the topmost frame.  For s390 the FP is setup
> after static stack allocation, so --fno-omit-frame-pointer would enforce
> FP==SP in any frame that does not perform dynamic stack allocation.

From your latest email, I take it I can ignore the above?

> 
> > +			goto done;
> > +		cfa = state->fp;
> > +	} else {
> > +		cfa = state->sp;
> > +	}
> > +
> > +	/* Get the Canonical Frame Address (CFA) */
> > +	cfa += frame->cfa_off;
> > +
> > +	/* stack going in wrong direction? */
> > +	if (cfa <= state->sp)
> > +		goto done;
> > +
> > +	/* Make sure that the address is word aligned */
> > +	shift = sizeof(long) == 4 ? 2 : 3;
> > +	if ((cfa + frame->ra_off) & ((1 << shift) - 1))
> > +		goto done;  
> 
> Do all architectures/ABI mandate register stack save slots to be aligned?
> s390 does.

I believe so.

> 
> > +
> > +	/* Find the Return Address (RA) */
> > +	if (get_user(ra, (unsigned long *)(cfa + frame->ra_off)))
> > +		goto done;
> > +  
> 
> Why not validate the FP stack save slot address as well?

You mean to validate cfa + frame->fp_off?

Isn't cfa the only real variable here? That is, if cfa + frame->ra_off
works, wouldn't the same go for frame->fp_off, as both frame->ra_off
and frame->fp_off are constants set by the architecture, and should be
word aligned.

-- Steve

> 
> > +	if (frame->fp_off && get_user(fp, (unsigned long __user *)(cfa + frame->fp_off)))
> > +		goto done;
> > +