On Sat, Jun 27, 2026 at 1:14 PM David Windsor <[email protected]> wrote: > > On Tue, Jun 23, 2026 at 9:25 AM Oleg Nesterov <[email protected]> wrote: > > > > On 06/23, Peter Zijlstra wrote: > > > > > > On Tue, Jun 23, 2026 at 02:52:32PM +0200, Oleg Nesterov wrote: > > > > On 06/22, David Windsor wrote: > > > > > > > > > > --- a/arch/x86/kernel/uprobes.c > > > > > +++ b/arch/x86/kernel/uprobes.c > > > > > @@ -1246,8 +1246,12 @@ static int default_post_xol_op(struct > > > > > arch_uprobe *auprobe, struct pt_regs *regs > > > > > long correction = utask->vaddr - utask->xol_vaddr; > > > > > regs->ip += correction; > > > > > } else if (auprobe->defparam.fixups & UPROBE_FIX_CALL) { > > > > > + unsigned long retaddr = utask->vaddr + > > > > > auprobe->defparam.ilen; > > > > > + > > > > > regs->sp += sizeof_long(regs); /* Pop incorrect return > > > > > address */ > > > > > - if (emulate_push_stack(regs, utask->vaddr + > > > > > auprobe->defparam.ilen)) > > > > > + if (emulate_push_stack(regs, retaddr)) > > > > > + return -ERESTART; > > > > > + if (shstk_update_last_frame(retaddr)) > > > > > return -ERESTART; > > > > > > > > Well, if shstk_update_last_frame() fails after emulate_push_stack(), we > > > > should > > > > probably return another error, so that the caller handle_singlestep() > > > > will kill > > > > this task? > > > > > > Makes sense, the other user has a force_sig(SIGSEGV) on failure. > > > > Offtopic question... both shstk_update_last_frame() and shstk_push() are > > only > > used by arch/x86/kernel/uprobes.c. But they are not symmetric in that > > shstk_update_last_frame() returns 0 if !features_enabled(ARCH_SHSTK_SHSTK), > > while shstk_push() returns -ENOTSUPP in this case. > > > > That is why the users can't just do "if (shstk_push(xxx)) ...". This is > > really > > minor, but perhaps it makes sense to change shstk_push() to return 0 in this > > case too? I don't think -ENOTSUPP is actually useful... > > > > Agreed, will send a follow-up patch changing shstk_push() to return 0 > rather than -ENOTSUPP. > > I'll send v2 shortly with the additional call to force_sig(SIGSEGV) to > balance out the callers. >
Actually, we don't need to force_sig() ourselves in this case, the handle_singlestep() path will take care of that. We just need to return something that's not -ERESTART. You may have meant that in your reply, but just noting it for the record for when I send v2. > > Oleg. > >
