Linux s390 Architecture development
 help / color / mirror / Atom feed
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

  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