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

On 10.07.2025 17:21, Steven Rostedt wrote:

On Wed, 9 Jul 2025 12:01:14 +0200
Jens Remus [off-list ref] wrote:

quoted

quoted

+	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?

Correct.

quoted

quoted

+	/* 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.

quoted

quoted

+
+	/* 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?

Yes.

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.

cfa + frame->ra_off could be aligned by chance.  So could
cfa + frame->fp_off be as well of course.

On s390 the CFA must be aligned (as the SP must be aligned) and the
FP and RA offsets from CFA must be aligned, as pointer / 64-bit integers
(such as 64-bit register values) must be aligned as well.

So the CFA (and/or offset), FP offset, and RA offset could be validated
individually.  Not sure if that would be over engineering though.

quoted

quoted

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

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/