Re: [PATCH v14 13/19] unwind_user: Enable archs that pass RA in a register

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

On 5/19/2026 2:42 PM, Steven Rostedt wrote:
> On Tue, 19 May 2026 14:24:34 +0200
> Jens Remus <jremus@linux.ibm.com> wrote:
> 
>>> How can this be triggered? Does it require an architecture to implement
>>> only partial sframe to trigger?  
>>
>> On x86-64 it can be triggered from user space using forged .sframe that
>> incorrectly specifies no fixed RA offset and thus causes the common
>> unwind user (sframe) logic to recover RA from the RA register using 
>> the dummy unwind_user_get_ra_reg().
> 
> OK, so if user space could trigger it via a invalid .sframe, I would do at
> most a pr_debug() (may not even need a 'once'), and remove that this task
> has an sframe. That is, as soon as a sframe is discovered to be corrupted,
> it should be discarded and never used after that.


Removing the .sframe section is not straightforward in this specific
case, as the issue is not detected in the sframe decoding but only later
in unwind_user_next_common() once it calls the dummy
unwind_user_get_ra_reg().  unwind_user_next_common() gets called by
unwind_user_next_sframe() after it obtained the information to unwind
the frame using sframe_find().  Therefore the offending struct
sframe_section is no longer known.  A reference to it cannot be stored
in struct unwind_user_frame, as that would require a SRCU read lock to
be held.  Instead the sec->sframe_start could be stored in struct
unwind_user_frame, which could then be passed to sframe_remove_section().
Instead of adding sframe-specific information to struct unwind_user_frame
I would prefer to use the state->ip as key to remove the offending
.sframe section.  Below is a compile-tested patch that outlines how
that could potentially work.  The sframe decoding cannot detect this
as it would need a mean to know whether the architecture implements
unwind_user_get_ra_reg() (and likewise for unwind_user_get_reg()).

Is there a simpler solution?


diff --git a/include/linux/sframe.h b/include/linux/sframe.h
@@ -33,6 +33,7 @@ extern void sframe_free_mm(struct mm_struct *mm);
 extern int sframe_add_section(unsigned long sframe_start, unsigned long sframe_end,
                              unsigned long text_start, unsigned long text_end);
 extern int sframe_remove_section(unsigned long sframe_addr);
+extern int sframe_remove_section_for_ip(unsigned long ip);
 extern int sframe_find(unsigned long ip, struct unwind_user_frame *frame);

 static inline bool current_has_sframe(void)
@@ -52,6 +53,7 @@ static inline int sframe_add_section(unsigned long sframe_start, unsigned long s
        return -ENOSYS;
 }
 static inline int sframe_remove_section(unsigned long sframe_addr) { return -ENOSYS; }
+extern int sframe_remove_section_for_ip(unsigned long ip) { return -ENOSYS; }
 static inline int sframe_find(unsigned long ip, struct unwind_user_frame *frame) { return -ENOSYS; }
 static inline bool current_has_sframe(void) { return false; }

diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
@@ -21,7 +21,7 @@ static inline int unwind_user_get_ra_reg(unsigned long *val)
 {
        pr_debug_once("%s (%d): unwind_user_get_ra_reg() not implemented\n",
                      current->comm, current->pid);
-       return -EINVAL;
+       return -EFAULT;
 }
 #define unwind_user_get_ra_reg unwind_user_get_ra_reg
 #endif
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
@@ -886,6 +886,20 @@ int sframe_remove_section(unsigned long sframe_start)
        return 0;
 }

+int sframe_remove_section_for_ip(unsigned long ip)
+{
+       struct mm_struct *mm = current->mm;
+       struct sframe_section *sec;
+
+       guard(srcu)(&sframe_srcu);
+
+       sec = mtree_load(&mm->sframe_mt, ip);
+       if (!sec)
+               return -EINVAL;
+
+       return sframe_remove_section(sec->sframe_start);
+}
+
 void sframe_free_mm(struct mm_struct *mm)
 {
        struct sframe_section *sec;
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
@@ -45,6 +45,7 @@ static int unwind_user_next_common(struct unwind_user_state *state,
                                   const struct unwind_user_frame *frame)
 {
        unsigned long cfa, sp, fp, ra;
+       int ret;

        /* Stop unwinding when reaching an outermost frame. */
        if (frame->outermost) {
@@ -94,8 +95,11 @@ static int unwind_user_next_common(struct unwind_user_state *state,
        /* Get the Return Address (RA) */
        switch (frame->ra.rule) {
        case UNWIND_USER_RULE_RETAIN:
-               if (!state->topmost || unwind_user_get_ra_reg(&ra))
+               if (!state->topmost)
                        return -EINVAL;
+               ret = unwind_user_get_ra_reg(&ra);
+               if (ret)
+                       return ret;
                break;
        case UNWIND_USER_RULE_CFA_OFFSET:
                /*
@@ -173,11 +177,16 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
 static int unwind_user_next_sframe(struct unwind_user_state *state)
 {
        struct unwind_user_frame frame;
+       unsigned long ip = state->ip;
+       int ret;

        /* sframe expects the frame to be local storage */
-       if (sframe_find(state->ip, &frame))
+       if (sframe_find(ip, &frame))
                return -ENOENT;
-       return unwind_user_next_common(state, &frame);
+       ret = unwind_user_next_common(state, &frame);
+       if (ret == -EFAULT)
+               sframe_remove_section_for_ip(ip);
+       return ret;
 }

 static int unwind_user_next(struct unwind_user_state *state)


> 
> A print could be done just to inform the admin of what happened. And since
> the sframe would be removed, that prevents it from spamming the console, so
> the "once" isn't needed. We may want to show it for other applications.
> 
> The print should state the comm and pid of the task that failed.
Thanks and regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

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