Re: [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC

[Ilya Leoshkevich <iii@linux.ibm.com>]

On 8/28/26 16:09, sashiko-bot@kernel.org wrote:
> 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 */
> 

This is a naming and not a functional issue. The IBM037 punctuation
marks must be present in the table for the series to make sense.

I guess a 100% proper name here would be something along the lines of
"invariant with IBM037 punctuation" or "IBM037 without weird
characters", but I don't think it's that important: one can look up
the Wikipedia reference in the commit message to get to the exact table
I used here.

I will defer it to the maintainers, whether I need to change comments,
names, or the commit message here.