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, "Heiko Carstens" <hca@linux.ibm.com>,
	"Vasily Gorbik" <gor@linux.ibm.com>,
	"Alexander Gordeev" <agordeev@linux.ibm.com>,
	"Christian Borntraeger" <borntraeger@linux.ibm.com>
Subject: Re: [PATCH v8 02/12] syscalls: Create unified partial table for all archs
Date: Sat, 19 Sep 2026 00:41:56 +0000	[thread overview]
Message-ID: <20260919004157.244001F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260918-tonyk-syscall_table-v8-2-0e2d60cad13e@igalia.com>

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] syscall tables: validation breaks for architectures with syscall numbers > 472
- [Medium] syscall tables: build fails under set -e if grep matches no ABIs
- [Medium] syscall tables: default execution without --abis breaks on duplicate clone3
- [Low] syscall tables: temporary file leak on script failure
--

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

syscalls: Create unified partial table for all archs

Creates a new unified syscall table and modifies generation scripts to use it.
Introduces special ABIs for clone3 variants.

> diff --git a/scripts/syscall_common.tbl b/scripts/syscall_common.tbl
> new file mode 100644
> index 0000000000000..8085030eb869e
> --- /dev/null
> +++ b/scripts/syscall_common.tbl
[ ... ]
> +434	common	pidfd_open			sys_pidfd_open
> +435	clone3	clone3				sys_clone3
> +435	__clone3	clone3			__sys_clone3

[Severity: Medium]
Will default execution without an explicit ABI filter break on these duplicate
clone3 entries?

The script permits omitting the --abis flag, which falls back to matching
all lines. Processing both of these mutually exclusive entries causes
syscalltbl.sh to erroneously fail its duplicate number validation.

[ ... ]
> diff --git a/scripts/syscallhdr.sh b/scripts/syscallhdr.sh
> index bad1ab606a0fb..8f7dfcf19a4c5 100755
> --- a/scripts/syscallhdr.sh
> +++ b/scripts/syscallhdr.sh
[ ... ]
> +max=0
> +# gen_hdr(infile)
> +gen_hdr() {
> +	input=$1
> +	tmpfile=$(mktemp)

[Severity: Low]
Does this temporary file leak on script failure? File creation occurs without
a guaranteed cleanup mechanism on error paths.

>  
> -	max=0
> -	while read nr abi name native compat ; do
> +	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$input" > $tmpfile

[Severity: Medium]
Since this script uses set -e, will executing grep as a standalone command
uncontrollably abort the script if no matching lines are found?

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

[Severity: Medium]
Does this unconditionally overwrite max, discarding higher syscall numbers seen
in previously processed architecture-specific tables?

[ ... ]
> diff --git a/scripts/syscalltbl.sh b/scripts/syscalltbl.sh
> index c4b1f85c2dd63..70da0039ab22f 100755
> --- a/scripts/syscalltbl.sh
> +++ b/scripts/syscalltbl.sh
[ ... ]
>  nxt=0
>  
> -grep -E "^[0-9]+[[:space:]]+$abis" "$infile" | {
> +# gen_tbl(infile)
> +gen_tbl() {
> +	input=$1
> +	tmpfile=$(mktemp)

[Severity: Low]
Does this temporary file leak if the script aborts early? There is no
cleanup mechanism on error paths.

>  
> +	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$input" > $tmpfile

[Severity: Medium]
If this grep finds no matches, will it exit with 1 and instantly kill the
build script under set -e?

>  	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: Medium]
Does the global nxt state bleed across sequentially processed files?

If an architecture that has syscall numbers greater than the highest number in
the common table uses this script with --common-tbl, sequential processing
starts at a lower number than the end of the previous arch table, causing
this sorted order validation to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-tonyk-syscall_table-v8-0-0e2d60cad13e@igalia.com?part=2

  reply	other threads:[~2026-09-19  0:41 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-19  0:31 [PATCH v8 00/12] syscalls: Add a shared table for all archs André Almeida
2026-09-19  0:31 ` [PATCH v8 01/12] syscalls: Make --abis parsing more robust André Almeida
2026-09-19  0:41   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 02/12] syscalls: Create unified partial table for all archs André Almeida
2026-09-19  0:41   ` sashiko-bot [this message]
2026-09-19  0:31 ` [PATCH v8 03/12] scripts/syscall.tbl: Use the common table André Almeida
2026-09-19  0:41   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 04/12] s390: Use the common syscall table André Almeida
2026-09-19  0:50   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 05/12] sparc: " André Almeida
2026-09-19  0:45   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 06/12] mips: Remove duplicated syscallnr.sh André Almeida
2026-09-19  0:45   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 07/12] mips: Get rid of custom mips ABIs for syscall tables André Almeida
2026-09-19  0:52   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 08/12] mips: Use the common syscall table André Almeida
2026-09-19  0:54   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 09/12] syscalls: Add an option for offsetting the common table André Almeida
2026-09-19  0:55   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 10/12] alpha: Define entry point for set_mempolicy_home_node syscall André Almeida
2026-09-19  0:55   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 11/12] alpha: Remove alpha_ prefix from custom syscall entries André Almeida
2026-09-19  0:59   ` sashiko-bot
2026-09-19  0:31 ` [PATCH v8 12/12] alpha: Use the common syscall table André Almeida
2026-09-19  1:01   ` sashiko-bot

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=20260919004157.244001F000FF@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.