All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "André Almeida" <andrealmeid@igalia.com>
Cc: linux-s390@vger.kernel.org,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>,
	"Heiko Carstens" <hca@linux.ibm.com>
Subject: Re: [PATCH v7 02/13] syscalls: Create unified partial table for all archs
Date: Thu, 17 Sep 2026 23:57:13 +0000	[thread overview]
Message-ID: <20260917235714.1DD9E1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260917-tonyk-syscall_table-v7-2-f4cd8f8e3265@igalia.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] scripts: unconditional max assignment in gen_hdr() truncates syscall bounds
- [Low] scripts: global nxt variable in gen_tbl() prevents processing overlapping tables
- [Low] scripts: unhandled grep exit status under set -e aborts build and leaks files
--

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

syscalls: Create unified partial table for all archs

This commit creates a unified partial table for syscalls shared across all
architectures. It adapts generation scripts to use this table, allowing every
new syscall to be added to a single file. It also creates special ABIs and
Makefiles flags to support different architectural entry points.

> diff --git a/scripts/syscallhdr.sh b/scripts/syscallhdr.sh
> index bad1ab606a0fb..8f7dfcf19a4c5 100755
> --- a/scripts/syscallhdr.sh
> +++ b/scripts/syscallhdr.sh
[ ... ]
> @@ -69,13 +76,35 @@ guard=_UAPI_ASM_$(basename "$outfile" |
>  	sed -e 'y/abcdefghijklmnopqrstuvwxyz/ABCDEFGHIJKLMNOPQRSTUVWXYZ/' \
>  	-e 's/[^A-Z0-9_]/_/g' -e 's/__/_/g')
[ ... ]
> +max=0
> +# gen_hdr(infile)
> +gen_hdr() {
> +	input=$1
> +	tmpfile=$(mktemp)
>  
> -	max=0
> -	while read nr abi name native compat ; do
> +	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$input" > $tmpfile

[Severity: Low]
Does this unhandled grep exit status abort the build and leak files?

Since this script runs under set -e, if the specified ABI has no entries in the
parsed table, grep returns exit status 1.

Due to set -e, the shell will immediately kill the script, bypassing the
rm $tmpfile cleanup at the end of the function.

> +	while read nr abi name native compat; do
>  
>  		max=$nr

[Severity: Medium]
Will this unconditional max assignment truncate the syscall bounds?

gen_hdr() sequentially processes the arch-specific table and then the common
table. The global max variable is unconditionally assigned max=$nr.

Because the common table is processed last, its maximum number will permanently
overwrite any higher maximum previously read from the arch table.

>  
> @@ -84,15 +113,21 @@ grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$infile" | {
>  		fi
>  
>  		echo "#define __NR_$prefix$name $nr"
> -	done
>  
> -	if [ -n "$emit_nr" ]; then
> -		echo
> -		echo "#ifdef __KERNEL__"
> -		echo "#define __NR_${prefix}syscalls $(($max + 1))"
> -		echo "#endif"
> -	fi
> +	done < $tmpfile
> +	rm $tmpfile
> +}
[ ... ]
> diff --git a/scripts/syscalltbl.sh b/scripts/syscalltbl.sh
> index c4b1f85c2dd63..70da0039ab22f 100755
> --- a/scripts/syscalltbl.sh
> +++ b/scripts/syscalltbl.sh
[ ... ]
> @@ -52,12 +59,16 @@ outfile="$2"
>  
>  nxt=0
>  
> -grep -E "^[0-9]+[[:space:]]+$abis" "$infile" | {
> +# gen_tbl(infile)
> +gen_tbl() {
> +	input=$1
> +	tmpfile=$(mktemp)
>  
> +	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$input" > $tmpfile

[Severity: Low]
Can this unhandled grep exit status abort the build and leak files?

Similar to gen_hdr(), this script also runs under set -e. If the grep command
finds zero matches, it exits with 1, which causes set -e to kill the script
here before the cleanup routine can run.

>  	while read nr abi name native compat noreturn; do
>  
>  		if [ $nxt -gt $nr ]; then
> -			echo "error: $infile: syscall table is not sorted or duplicates the same syscall number" >&2
> +			echo "error: $input: syscall table is not sorted or duplicates the same syscall number" >&2
>  			exit 1
>  		fi

[Severity: Low]
Does this global nxt variable in gen_tbl() prevent processing overlapping
tables?

The validation check enforces a strict monotonic increase in syscall numbers.
gen_tbl() maintains a global nxt variable across sequential file invocations.

If the first file's maximum exceeds the second file's minimum, processing the
second file will immediately fail this validation check and exit.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917-tonyk-syscall_table-v7-0-f4cd8f8e3265@igalia.com?part=2

  reply	other threads:[~2026-09-17 23:57 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17 23:46 [PATCH v7 00/13] syscalls: Add a shared table for all archs André Almeida
2026-09-17 23:46 ` [PATCH v7 01/13] syscalls: Make --abis parsing more robust André Almeida
2026-09-17 23:52   ` sashiko-bot
2026-09-17 23:46 ` [PATCH v7 02/13] syscalls: Create unified partial table for all archs André Almeida
2026-09-17 23:57   ` sashiko-bot [this message]
2026-09-17 23:46 ` [PATCH v7 03/13] scripts/syscall.tbl: Use the common table André Almeida
2026-09-18  0:02   ` sashiko-bot
2026-09-18  6:34   ` Arnd Bergmann
2026-09-18 19:58     ` André Almeida
2026-09-17 23:46 ` [PATCH v7 04/13] arm: Use the common syscall table André Almeida
2026-09-18  0:01   ` sashiko-bot
2026-09-18  6:22   ` Arnd Bergmann
2026-09-18 20:10     ` André Almeida
2026-09-17 23:46 ` [PATCH v7 05/13] s390: " André Almeida
2026-09-17 23:56   ` sashiko-bot
2026-09-17 23:46 ` [PATCH v7 06/13] sparc: " André Almeida
2026-09-17 23:53   ` sashiko-bot
2026-09-17 23:46 ` [PATCH v7 07/13] mips: Remove duplicated syscallnr.sh André Almeida
2026-09-17 23:51   ` sashiko-bot
2026-09-17 23:46 ` [PATCH v7 08/13] mips: Get rid of custom mips ABIs for syscall tables André Almeida
2026-09-17 23:54   ` sashiko-bot
2026-09-17 23:46 ` [PATCH v7 09/13] mips: Use the common syscall table André Almeida
2026-09-17 23:55   ` sashiko-bot
2026-09-18  6:56   ` Arnd Bergmann
2026-09-17 23:46 ` [PATCH v7 10/13] syscalls: Add an option for offsetting the common table André Almeida
2026-09-17 23:57   ` sashiko-bot
2026-09-17 23:46 ` [PATCH v7 11/13] alpha: Define entry point for set_mempolicy_home_node syscall André Almeida
2026-09-17 23:59   ` sashiko-bot
2026-09-18 11:36   ` Magnus Lindholm
2026-09-17 23:46 ` [PATCH v7 12/13] alpha: Remove alpha_ prefix from custom syscall entries André Almeida
2026-09-17 23:59   ` sashiko-bot
2026-09-18 10:54   ` Magnus Lindholm
2026-09-18 20:11     ` André Almeida
2026-09-17 23:46 ` [PATCH v7 13/13] alpha: Use the common syscall table André Almeida
2026-09-17 23:59   ` sashiko-bot
2026-09-18 10:57   ` Magnus Lindholm

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260917235714.1DD9E1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=andrealmeid@igalia.com \
    --cc=borntraeger@linux.ibm.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=linux-s390@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.