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

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

quoted

 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?

quoted

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

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?

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

quoted

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