From: sashiko-bot@kernel.org
To: "Ilya Leoshkevich" <iii@linux.ibm.com>
Cc: "Christian Borntraeger" <borntraeger@linux.ibm.com>,
linux-s390@vger.kernel.org, "Vasily Gorbik" <gor@linux.ibm.com>,
"Alexander Gordeev" <agordeev@linux.ibm.com>,
"Heiko Carstens" <hca@linux.ibm.com>
Subject: Re: [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC
Date: Fri, 28 Aug 2026 14:09:45 +0000 [thread overview]
Message-ID: <20260828140945.EAA981F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260828140057.1728173-2-iii@linux.ibm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `_ebctype_inv` maps variant EBCDIC characters as invariant punctuation, violating the definition of the invariant subset.
--
commit 4c4158381dc4e63b906d199cf0d49673aee1a203
Author: Ilya Leoshkevich <iii@linux.ibm.com>
s390/ebcdic: Add character classes for the invariant subset of EBCDIC
Add _ctypes-like array for the invariant subset of EBCDIC, which
consists of characters that should have the same assignments on all
EBCDIC code pages that use the Latin alphabet [1].
> diff --git a/arch/s390/kernel/ebcdic.c b/arch/s390/kernel/ebcdic.c
> index 0e51fa537262b..15cbb0b42aa6e 100644
> --- a/arch/s390/kernel/ebcdic.c
> +++ b/arch/s390/kernel/ebcdic.c
> @@ -392,10 +392,48 @@ __u8 _ebc_toupper[256] =
[ ... ]
> +/*
> + * EBCDIC -> character classes
> + */
> +const __u8 _ebctype_inv[256] = {
> + _C, _C, _C, _C, _C, _C|_S, _C, _C, /* 0x00-0x07 */
> + _C, _C, _C, _C|_S, _C|_S, _C|_S, _C, _C, /* 0x08-0x0f */
> + _C, _C, _C, _C, _C, _C|_S, _C, _C, /* 0x10-0x17 */
> + _C, _C, _C, _C, _C, _C, _C, _C, /* 0x18-0x1f */
> + _C, _C, _C, _C, _C, _C|_S, _C, _C, /* 0x20-0x27 */
> + _C, _C, _C, _C, _C, _C, _C, _C, /* 0x28-0x2f */
> + 0, 0, _C, _C, _C, _C, _C, _C, /* 0x30-0x37 */
> + _C, _C, _C, _C, _C, _C, 0, _C, /* 0x38-0x3f */
> + _S|_SP, 0, 0, 0, 0, 0, 0, 0, /* 0x40-0x47 */
> + 0, 0, _P, _P, _P, _P, _P, _P, /* 0x48-0x4f */
[Severity: Medium]
Does this array incorrectly include variant EBCDIC characters?
0x4A is mapped as _P (invariant punctuation) here, but isn't it a variant
character (e.g., '¢' in US 037, '[' in German 273)?
If a kernel command line contains these bytes, they might be incorrectly
accepted as valid invariant EBCDIC.
> + _P, 0, 0, 0, 0, 0, 0, 0, /* 0x50-0x57 */
> + 0, 0, _P, _P, _P, _P, _P, _P, /* 0x58-0x5f */
> + _P, _P, 0, 0, 0, 0, 0, 0, /* 0x60-0x67 */
> + 0, 0, _P, _P, _P, _P, _P, _P, /* 0x68-0x6f */
> + 0, 0, 0, 0, 0, 0, 0, 0, /* 0x70-0x77 */
> + 0, _P, _P, _P, _P, _P, _P, _P, /* 0x78-0x7f */
> + 0, _L|_X, _L|_X, _L|_X, _L|_X, _L|_X, _L|_X, _L, /* 0x80-0x87 */
> + _L, _L, 0, 0, 0, 0, 0, _P, /* 0x88-0x8f */
> + 0, _L, _L, _L, _L, _L, _L, _L, /* 0x90-0x97 */
> + _L, _L, 0, 0, 0, 0, 0, 0, /* 0x98-0x9f */
> + 0, _P, _L, _L, _L, _L, _L, _L, /* 0xa0-0xa7 */
> + _L, _L, 0, 0, 0, 0, 0, 0, /* 0xa8-0xaf */
> + _P, 0, 0, 0, 0, 0, 0, 0, /* 0xb0-0xb7 */
> + 0, 0, _P, _P, 0, 0, 0, 0, /* 0xb8-0xbf */
> + _P, _U|_X, _U|_X, _U|_X, _U|_X, _U|_X, _U|_X, _U, /* 0xc0-0xc7 */
[Severity: Medium]
Is 0xC0 also a variant character?
It is classified as _P here, but it can represent '{' in US 037 or 'Ä'
in German 273.
This seems to violate the definition of the invariant subset and could lead
to missed warnings for garbled boot parameters.
> + _U, _U, 0, 0, 0, 0, 0, 0, /* 0xc8-0xcf */
> + _P, _U, _U, _U, _U, _U, _U, _U, /* 0xd0-0xd7 */
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260828140057.1728173-1-iii@linux.ibm.com?part=1
next prev parent reply other threads:[~2026-08-28 14:09 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-28 13:59 [PATCH v3 0/2] s390: Warn if kernel command line contains non-printable EBCDIC Ilya Leoshkevich
2026-08-28 13:59 ` [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC Ilya Leoshkevich
2026-08-28 14:09 ` sashiko-bot [this message]
2026-08-28 13:59 ` [PATCH v3 2/2] s390: Warn if kernel command line contains non-printable EBCDIC characters Ilya Leoshkevich
2026-08-28 14:22 ` 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=20260828140945.EAA981F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=iii@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox