Re: [PATCH v4 19/39] unwind_user/sframe: Add support for reading .sframe contents

Josh Poimboeuf
2025-02-04T18:51:24Z

On Thu, Jan 30, 2025 at 04:47:00PM +0100, Jens Remus wrote:
> On 22.01.2025 03:31, Josh Poimboeuf wrote:
> > +struct sframe_fre {
> > + unsigned int size;
> > + s32 ip_off;
>
> The IP offset (from function start) in the SFrame V2 FDE is unsigned:
>
> u32 ip_off;

Indeed.

> > +#define __UNSAFE_GET_USER_INC(to, from, type, label) \
> > +({ \
> > + type __to; \
> > + unsafe_get_user(__to, (type __user *)from, label); \
> > + from += sizeof(__to); \
> > + to = (typeof(to))__to; \
> > +})
> > +
> > +#define UNSAFE_GET_USER_INC(to, from, size, label) \
> > +({ \
> > + switch (size) { \
> > + case 1: \
> > + __UNSAFE_GET_USER_INC(to, from, u8, label); \
> > + break; \
> > + case 2: \
> > + __UNSAFE_GET_USER_INC(to, from, u16, label); \
> > + break; \
> > + case 4: \
> > + __UNSAFE_GET_USER_INC(to, from, u32, label); \
> > + break; \
> > + default: \
> > + return -EFAULT; \
> > + } \
> > +})
>
> This does not work for the signed SFrame fields, such as the FRE CFA,
> RA, and FP offsets, as it does not perform the required sign extension.
> One option would be to rename to UNSAFE_GET_USER_UNSIGNED_INC() and
> re-introduce UNSAFE_GET_USER_SIGNED_INC() using s8, s16, and s32.

See the following line in __UNSAFE_GET_USER_INC():

to = (typeof(to))__to;

Does that not do the sign extension?

--
Josh