Hi, Hengqi,
On Wed, Nov 22, 2023 at 3:34 PM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
>
> Hi, Huacai,
>
> On Wed, Nov 22, 2023 at 2:32 PM Huacai Chen <chenhuacai@xxxxxxxxxx> wrote:
> >
> > Hi, Hengqi,
> >
> > On Wed, Nov 22, 2023 at 1:14 PM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
> > >
> > > Currently, we store syscall number in pt_regs::regs[11] and it may be
> > > changed during syscall execution. Take `execve` as an example:
> > >
> > > sys_execve
> > > -> do_execve
> > > -> do_execveat_common
> > > -> bprm_execve
> > > -> exec_binprm
> > > -> search_binary_handler
> > > -> load_elf_binary
> > > -> ELF_PLAT_INIT
> > >
> > > ELF_PLAT_INIT reset regs[11] to 0, later in syscall_exit_to_user_mode
> > > we get a wrong syscall nr.
> > >
> > > Known affected syscalls includes execve/execveat/rt_sigreturn. Tools
> > > like execsnoop do not work properly because the sys_exit_* tracepoints
> > > does not trigger at all.
> > >
> > > Let's store syscall nr in thread_info instead.
> > Can we just modify ELF_PLAT_INIT and not clear regs[11]?
> >
>
> I am uncertain about the side effects of changing ELF_PLAT_INIT.
> From a completeness perspective, changing ELF_PLAT_INIT is suboptimal,
> rt_sigreturn is affected in another code path, and there may be other
> syscalls that I am unaware of.
Save syscall number in thread_info has more side effects, because
ptrace allows us to change the number during syscall, then we should
keep consistency between syscall and regs[11].
And about ELF_PLAT_INIT, maybe Arnd can give us some more information.
Hi, Arnd,
I found some new architectures, such as ARM64 and RISC-V, just do
nearly nothing in ELF_PLAT_INIT, while some old architectures, such as
x86 and MIPS, clear most of the registers, do you know why?
Huacai
>
> > Huacai
> >
> > >
> > > Fixes: be769645a2aef ("LoongArch: Add system call support")
> > > Signed-off-by: Hengqi Chen <hengqi.chen@xxxxxxxxx>
> > > ---
> > > arch/loongarch/include/asm/syscall.h | 2 +-
> > > arch/loongarch/kernel/syscall.c | 1 +
> > > 2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/loongarch/include/asm/syscall.h b/arch/loongarch/include/asm/syscall.h
> > > index e286dc58476e..2317d674b92a 100644
> > > --- a/arch/loongarch/include/asm/syscall.h
> > > +++ b/arch/loongarch/include/asm/syscall.h
> > > @@ -23,7 +23,7 @@ extern void *sys_call_table[];
> > > static inline long syscall_get_nr(struct task_struct *task,
> > > struct pt_regs *regs)
> > > {
> > > - return regs->regs[11];
> > > + return task_thread_info(task)->syscall;
> > > }
> > >
> > > static inline void syscall_rollback(struct task_struct *task,
> > > diff --git a/arch/loongarch/kernel/syscall.c b/arch/loongarch/kernel/syscall.c
> > > index b4c5acd7aa3b..2783e33cf276 100644
> > > --- a/arch/loongarch/kernel/syscall.c
> > > +++ b/arch/loongarch/kernel/syscall.c
> > > @@ -52,6 +52,7 @@ void noinstr do_syscall(struct pt_regs *regs)
> > > regs->orig_a0 = regs->regs[4];
> > > regs->regs[4] = -ENOSYS;
> > >
> > > + task_thread_info(current)->syscall = nr;
> > > nr = syscall_enter_from_user_mode(regs, nr);
> > >
> > > if (nr < NR_syscalls) {
> > > --
> > > 2.42.0
> > >
