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

[Jens Remus <jremus@linux.ibm.com>]

On 10.07.2025 17:21, Steven Rostedt wrote:
> On Wed, 9 Jul 2025 12:01:14 +0200
> Jens Remus <jremus@linux.ibm.com> wrote:

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

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

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.

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