Re: [PATCH v9 13/15] syscalls: Avoid matching substrings in the ABI parsing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] syscalls: gen_hdr() and gen_tbl() regex breaks empty --abis and
  repeats $abis
--

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

syscalls: Avoid matching substrings in the ABI parsing

This commit updates the grep regular expressions in scripts/syscallhdr.sh
and scripts/syscalltbl.sh to require trailing whitespace after the ABI
pattern so that ABI names do not match as substrings, and appends "|| true"
to prevent the scripts from exiting early when grep finds no matches.

> diff --git a/scripts/syscallhdr.sh b/scripts/syscallhdr.sh
> index cf6bba3019b12..cf35ff05b8242 100755
> --- a/scripts/syscallhdr.sh
> +++ b/scripts/syscallhdr.sh
> @@ -109,7 +109,7 @@ gen_hdr() {
>  	tmpfile=$(mktemp)
>  	offset_nr=$2
>  
> -	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$input" > $tmpfile
> +	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis+[[:space:]]" "$input" > $tmpfile || true
>  	while read nr abi name native compat; do
>  
>  		if [ -n "$offset_nr" ]; then
> diff --git a/scripts/syscalltbl.sh b/scripts/syscalltbl.sh
> index 1e9cac32c02d2..403a604e7c760 100755
> --- a/scripts/syscalltbl.sh
> +++ b/scripts/syscalltbl.sh
> @@ -70,7 +70,7 @@ gen_tbl() {
>  	tmpfile=$(mktemp)
>  	offset_nr=$2
>  
> -	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis" "$input" > $tmpfile
> +	grep -E "^[0-9A-Fa-fXx]+[[:space:]]+$abis+[[:space:]]" "$input" > $tmpfile || true

[Severity: High]
What happens in gen_hdr() in scripts/syscallhdr.sh and gen_tbl() in
scripts/syscalltbl.sh when --abis is not passed on the command line?

Both scripts initialize abis to an empty string by default when --abis is
omitted:

    # default unless specified by options
    abis=

Architectures such as m68k, microblaze, sh, and xtensa invoke syscallhdr.sh
and syscalltbl.sh without --abis, for example in
arch/m68k/kernel/syscalls/Makefile:

    cmd_syshdr = $(CONFIG_SHELL) $(syshdr) --emit-nr $< $@

    quiet_cmd_systbl = SYSTBL  $@
          cmd_systbl = $(CONFIG_SHELL) $(systbl) $< $@

When abis is empty, the pattern in gen_hdr() and gen_tbl() expands to:

    ^[0-9A-Fa-fXx]+[[:space:]]++[[:space:]]

Because there is no subpattern to match the ABI column (such as common) and
the syscall number is separated from the ABI name by a single tab character,
grep matches zero lines (or rejects ++ as an invalid ERE repetition). With
"|| true" suppressing the non-zero exit status from grep, wouldn't gen_hdr()
and gen_tbl() read an empty tmpfile and silently generate empty syscall
headers and syscall tables for those architectures?

Also, when --abis is specified, the option parser constructs a parenthesized
group:

    --abis)
        abis="($(echo "${2%,}" | tr ',' '|'))"

Writing "$abis+[[:space:]]" places the "+" quantifier on the "$abis" group
rather than on "[[:space:]]", expanding to "(abc)+[[:space:]]" or
"(common|64)+[[:space:]]", which matches repeated or concatenated ABI names
such as "abcabc" or "common64".

Should abis instead default to a pattern that matches any non-whitespace
token (such as "[^[:space:]]+"), or should the "[[:space:]]+" suffix be
included inside abis only when --abis is set?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105-tonyk-syscall_table-v9-0-8d1275885d9c@igalia.com?part=13