Re: [PATCH v5 10/13] syscalls: Add an option for offsetting the common table

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The __NR_syscalls macro is calculated incorrectly when --common-offset is used, leading to a severely truncated syscall table bound.
- [Low] Dead and potentially dangerous code block referencing an undefined `offset` variable in `syscalltbl.sh` due to a copy-paste error.
- [Low] Inconsistent indentation in `syscalltbl.sh` using spaces instead of tabs.
--

--- Patch [10]: [PATCH v5 10/13] syscalls: Add an option for offsetting the common table ---
commit ca05bec6578b785868c677c2b401096b6deb225e
Author: André Almeida <[email protected]>

syscalls: Add an option for offsetting the common table

This commit adds a new --common-offset option to the syscall header and table
generation scripts. It allows shifting the syscall numbers for a common table
by a given value, facilitating reuse for architectures like alpha.

> diff --git a/scripts/syscallhdr.sh b/scripts/syscallhdr.sh
> index 54e09d3e686c4..ce6819111c27d 100755
> --- a/scripts/syscallhdr.sh
> +++ b/scripts/syscallhdr.sh

[ ... ]

> @@ -98,18 +103,24 @@ emit_end_guard() {
>  }
>  
>  max=0
> -# gen_hdr(infile)
> +# gen_hdr(infile, offset_nr)
>  gen_hdr() {
>  	input=$1
> +	offset_nr=$2
>  
>  	while read nr abi name native compat; do
>  
>  		max=$nr
>  
> +		if [ -n "$offset_nr" ]; then
> +			nr=$((nr + offset_nr))
> +		fi

[Severity: High]
Will max be assigned the un-offset syscall number before the offset is
applied?

If max is calculated before the addition, the ultimately emitted __NR_syscalls
macro could be smaller than the highest offsetted syscall number. Architectures
using this to size their syscall tables or bound-check userspace calls might
reject valid syscalls.

> +
>  		if [ -n "$offset" ]; then
>  			nr="($offset + $nr)"
>  		fi
>  

[ ... ]

> diff --git a/scripts/syscalltbl.sh b/scripts/syscalltbl.sh
> index 91c135143b22f..aea00834dd5ab 100755
> --- a/scripts/syscalltbl.sh
> +++ b/scripts/syscalltbl.sh

[ ... ]

> @@ -59,12 +64,21 @@ outfile="$2"
>  
>  nxt=0
>  
> -# gen_tbl(infile)
> +# gen_tbl(infile, offset_nr)
>  gen_tbl() {
>  	input=$1
> +	offset_nr=$2
>  
>  	while read nr abi name native compat noreturn; do
>  
> +                if [ -n "$offset_nr" ]; then
> +                        nr=$((nr + offset_nr))
> +                fi

[Severity: Low]
This isn't a bug, but it looks like these lines are indented with spaces
instead of tabs. The surrounding script appears to use tabs.

> +
> +		if [ -n "$offset" ]; then
> +			nr="($offset + $nr)"
> +		fi

[Severity: Low]
Is the offset variable defined in syscalltbl.sh?

It looks like the --offset argument is parsed in syscallhdr.sh but not in this
script. If an environment variable named offset happens to be set in the build
shell, could this cause a bash syntax error when injected into the arithmetic
context?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10