Re: [PATCH v6 09/13] mips: Use the common syscall table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Missing `--abis` flag in `cmd_sysnr` creates a latent out-of-bounds kernel memory access vulnerability on the syscall table.
- [Low] Typo in `rt_sigtimedwait` syscall entry causes linker error for N64 ABI.
- [Low] Typo in `ustat` syscall entry causes linker errors for both N32 and N64 ABIs.
- [Low] Build fatally broken due to missing and deleted prerequisites in Makefile rules for generated headers.
--

commit dbd1eadaff2c006f6b57d84d5f841a630aa58232
Author: André Almeida <andrealmeid@igalia.com>

mips: Use the common syscall table

This patch migrates MIPS to use the common syscall table from number 403
onwards, taking advantage of the shared numbers across architectures. It
merges the N32 and N64 syscall tables into a single syscall_n.tbl file and
updates the Makefile rules for generation, maintaining functional parity for
the O32, N32, and N64 ABIs.

> diff --git a/arch/mips/kernel/syscalls/Makefile b/arch/mips/kernel/syscalls/Makefile
> index 1bfc2346d1009..aaa5443ea9068 100644
> --- a/arch/mips/kernel/syscalls/Makefile
> +++ b/arch/mips/kernel/syscalls/Makefile
[ ... ]
>  quiet_cmd_sysnr = SYSNR   $@
>        cmd_sysnr = $(CONFIG_SHELL) $(sysnr) \
>  		  --prefix $(sysnr_pfx_$(basetarget))_Linux_ $< $@

[Severity: Medium]
Should this command also include the --abis flag like cmd_syshdr does? Since
cmd_sysnr runs on the common table without filtering by ABI, it seems it
might extract the highest syscall number across all ABIs. If a 32-bit-only
syscall is added later, couldn't this make __NR_64_Linux_syscalls larger than
the actual generated N64 table? 

A mismatch like this might allow an out-of-bounds read during the syscall
bounds check in scall64-n64.S. Also, because $(src_$*) is missing here, any
local syscalls wouldn't be counted either.

>  quiet_cmd_systbl = SYSTBL  $@
> -      cmd_systbl = $(CONFIG_SHELL) $(systbl) $< $@
> +      cmd_systbl = $(CONFIG_SHELL) $(systbl) \
> +		   --common-tbl $(systbl_common) \
> +		   --abis $(abis_$*) $(src_$*) $@
>  
>  $(uapi)/unistd_%.h: $(src)/syscall_%.tbl $(syshdr) FORCE
>  	$(call if_changed,syshdr)

[Severity: Low]
Does this rule need to be updated to use $(src_$*) instead of
$(src)/syscall_%.tbl? Since this patch deletes files like syscall_n32.tbl and
syscall_n64.tbl, Kbuild might fail with a "No rule to make target" error when
trying to build unistd_n32.h because it will still look for the deleted
tables.

[ ... ]
> -$(kapi)/syscall_table_%.h: $(src)/syscall_%.tbl $(systbl) FORCE
> +$(kapi)/syscall_table_%.h: $(systbl) $(systbl_common) FORCE
>  	$(call if_changed,systbl)

[Severity: Low]
Is it intentional to drop the local table variable $(src_$*) from the
prerequisites here? Without it, Kbuild might not trigger incremental rebuilds
if the local syscall table gets modified.

> diff --git a/arch/mips/kernel/syscalls/syscall_n.tbl b/arch/mips/kernel/syscalls/syscall_n.tbl
> new file mode 100644
> index 0000000000000..e3d278f715b7e
> --- /dev/null
> +++ b/arch/mips/kernel/syscalls/syscall_n.tbl
[ ... ]
> +124	common	capset				sys_capset
> +125	common	rt_sigpending			sys_rt_sigpending	compat_sys_rt_sigpending
> +126	common	rt_sigtimedwait			sys_rt_sigtimedwait_time	compat_sys_rt_sigtimedwait_time32

[Severity: Low]
Could there be a typo in the native entry point name here? The kernel function
is sys_rt_sigtimedwait, so sys_rt_sigtimedwait_time seems likely to cause an
unresolved symbol error when linking the N64 ABI.

> +127	common	rt_sigqueueinfo			sys_rt_sigqueueinfo	compat_sys_rt_sigqueueinfo
> +128	common	rt_sigsuspend			sys_rt_sigsuspend	compat_sys_rt_sigsuspend
> +129	common	sigaltstack			sys_sigaltstack		compat_sys_sigaltstack
> +130	common	utime				sys_utime		sys_utime32
> +131	common	mknod				sys_mknod
> +132	common	personality			sys_personality		sys_32_personality
> +133	common	ustat				sys_usta		compat_sys_usta

[Severity: Low]
Is it possible that the trailing 't' was accidentally truncated on these entry
points? Using sys_usta and compat_sys_usta instead of sys_ustat and
compat_sys_ustat will likely break the build with undefined references for
both N32 and N64 ABIs.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-tonyk-syscall_table-v6-0-fdd358456cf1@igalia.com?part=9