Linux s390 Architecture development
 help / color / mirror / Atom feed
* [PATCH v3 0/2] s390: Warn if kernel command line contains non-printable EBCDIC
@ 2026-08-28 13:59 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 13:59 ` [PATCH v3 2/2] s390: Warn if kernel command line contains non-printable EBCDIC characters Ilya Leoshkevich
  0 siblings, 2 replies; 7+ messages in thread
From: Ilya Leoshkevich @ 2026-08-28 13:59 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev
  Cc: David Laight, linux-s390, linux-kernel, Ilya Leoshkevich

v2: https://lore.kernel.org/all/20260826150702.949956-1-iii@linux.ibm.com/
v2 -> v3: Check whether source EBCDIC characters are printable
          (David, Heiko).

v1: https://lore.kernel.org/all/20260825150840.537653-1-iii@linux.ibm.com/
v1 -> v2: Account for newlines (Heiko).
          Minor stylistic improvements: inline int i, bool return type,
          comment modality.

Hi,

This series adds a warning for unintentional EBCDIC -> ASCII kernel
command line translations. Patch 1 adds EBCDIC character classes.
Patch 2 uses them for determining whether the command line is
printable.

Best regards,
Ilya

Ilya Leoshkevich (2):
  s390/ebcdic: Add character classes for the invariant subset of EBCDIC
  s390: Warn if kernel command line contains non-printable EBCDIC
    characters

 arch/s390/boot/ipl_parm.c      | 13 ++++++++++-
 arch/s390/include/asm/ebcdic.h |  5 +++++
 arch/s390/kernel/ebcdic.c      | 40 +++++++++++++++++++++++++++++++++-
 3 files changed, 56 insertions(+), 2 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC
  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 ` Ilya Leoshkevich
  2026-08-28 14:09   ` sashiko-bot
  2026-08-28 13:59 ` [PATCH v3 2/2] s390: Warn if kernel command line contains non-printable EBCDIC characters Ilya Leoshkevich
  1 sibling, 1 reply; 7+ messages in thread
From: Ilya Leoshkevich @ 2026-08-28 13:59 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev
  Cc: David Laight, linux-s390, linux-kernel, Ilya Leoshkevich

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].

[1] https://en.wikipedia.org/wiki/EBCDIC#Code_page_layout

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 arch/s390/include/asm/ebcdic.h |  5 +++++
 arch/s390/kernel/ebcdic.c      | 40 +++++++++++++++++++++++++++++++++-
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/arch/s390/include/asm/ebcdic.h b/arch/s390/include/asm/ebcdic.h
index 7164cb6584355..9b7e3a58ca738 100644
--- a/arch/s390/include/asm/ebcdic.h
+++ b/arch/s390/include/asm/ebcdic.h
@@ -10,6 +10,7 @@
 #ifndef _EBCDIC_H
 #define _EBCDIC_H
 
+#include <linux/ctype.h>
 #include <linux/types.h>
 
 extern __u8 _ascebc_500[256];   /* ASCII -> EBCDIC 500 conversion table */
@@ -18,6 +19,10 @@ extern __u8 _ascebc[256];   /* ASCII -> EBCDIC conversion table */
 extern __u8 _ebcasc[256];   /* EBCDIC -> ASCII conversion table */
 extern __u8 _ebc_tolower[256]; /* EBCDIC -> lowercase */
 extern __u8 _ebc_toupper[256]; /* EBCDIC -> uppercase */
+extern const __u8 _ebctype_inv[256]; /* EBCDIC -> character classes */
+
+#define __ismask_ebc_inv(x) (_ebctype_inv[(int)(unsigned char)(x)])
+#define isprint_ebc_inv(c) ((__ismask_ebc_inv(c)&(_P|_U|_L|_D|_SP)) != 0)
 
 static inline void
 codepage_convert(const __u8 *codepage, volatile char *addr, unsigned long nr)
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] =
 	0xF8, 0xF9, 0xFA, 0xFB, 0xFC, 0xFD, 0xFE, 0xFF
 };
 
+/*
+ * 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 */
+	_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 */
+	_U,     _U,    0,     0,     0,     0,     0,     0,	/* 0xc8-0xcf */
+	_P,     _U,    _U,    _U,    _U,    _U,    _U,    _U,	/* 0xd0-0xd7 */
+	_U,     _U,    0,     0,     0,     0,     0,     0,	/* 0xd8-0xdf */
+	_P,     0,     _U,    _U,    _U,    _U,    _U,    _U,	/* 0xe0-0xe7 */
+	_U,     _U,    0,     0,     0,     0,     0,     0,	/* 0xe8-0xef */
+	_D,     _D,    _D,    _D,    _D,    _D,    _D,    _D,	/* 0xf0-0xf7 */
+	_D,     _D,    0,     0,     0,     0,     0,     _C,	/* 0xf8-0xff */
+};
+
 EXPORT_SYMBOL(_ascebc_500);
 EXPORT_SYMBOL(_ebcasc_500);
 EXPORT_SYMBOL(_ascebc);
 EXPORT_SYMBOL(_ebcasc);
 EXPORT_SYMBOL(_ebc_tolower);
 EXPORT_SYMBOL(_ebc_toupper);
-
+EXPORT_SYMBOL(_ebctype_inv);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v3 2/2] s390: Warn if kernel command line contains non-printable EBCDIC characters
  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 13:59 ` Ilya Leoshkevich
  2026-08-28 14:22   ` sashiko-bot
  1 sibling, 1 reply; 7+ messages in thread
From: Ilya Leoshkevich @ 2026-08-28 13:59 UTC (permalink / raw)
  To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev
  Cc: David Laight, linux-s390, linux-kernel, Ilya Leoshkevich

Users may accidentally add multi-byte UTF-8 characters to zipl.conf
parmline, for example, by copying snippets containing non-breaking
spaces (\xC2\xA0) from web pages.

The kernel will then interpret the entire command line as EBCDIC,
making it unusable. Distinguish this situation from the legitimate
EBCDIC conversion by looking for non-printable characters and issue
a warning.

The current solution deliberately makes the following tradeoffs:

* Do not make this a hard failure, there may be a very small number
  of users who put characters with diacritics on their EBCDIC-encoded
  command lines.
* Do not use heuristics with arbitrary thresholds, these may fail
  intermittently on, e.g., punctuation-heavy command lines, and having
  to tune thresholds in subsequent patches is not desirable.
* Use the invariant subset of EBCDIC to determine whether characters
  are printable, the vast majority of command lines will contain
  characters from this subset.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 arch/s390/boot/ipl_parm.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/s390/boot/ipl_parm.c b/arch/s390/boot/ipl_parm.c
index 6bc950b92be76..24573307263ca 100644
--- a/arch/s390/boot/ipl_parm.c
+++ b/arch/s390/boot/ipl_parm.c
@@ -173,12 +173,23 @@ static inline int has_ebcdic_char(const char *str)
 	return 0;
 }
 
+static inline bool has_nonprintable_ebcdic_char(const char *str)
+{
+	for (; *str; str++)
+		if (!isprint_ebc_inv(*str))
+			return true;
+	return false;
+}
+
 void setup_boot_command_line(void)
 {
 	parmarea.command_line[COMMAND_LINE_SIZE - 1] = 0;
 	/* convert arch command line to ascii if necessary */
-	if (has_ebcdic_char(parmarea.command_line))
+	if (has_ebcdic_char(parmarea.command_line)) {
+		if (has_nonprintable_ebcdic_char(parmarea.command_line))
+			boot_warn("Kernel command line was treated as EBCDIC, but contains non-printable characters\n");
 		EBCASC(parmarea.command_line, COMMAND_LINE_SIZE);
+	}
 	/* copy arch command line */
 	strscpy(early_command_line, strim(parmarea.command_line));
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC
  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
  2026-08-31 12:14     ` Ilya Leoshkevich
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-28 14:09 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Christian Borntraeger, linux-s390, Vasily Gorbik,
	Alexander Gordeev, Heiko Carstens

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 2/2] s390: Warn if kernel command line contains non-printable EBCDIC characters
  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
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-28 14:22 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: Christian Borntraeger, Vasily Gorbik, Heiko Carstens, linux-s390,
	Alexander Gordeev

> Users may accidentally add multi-byte UTF-8 characters to zipl.conf
> parmline, for example, by copying snippets containing non-breaking
> spaces (\xC2\xA0) from web pages.
> 
> The kernel will then interpret the entire command line as EBCDIC,
> making it unusable. Distinguish this situation from the legitimate
> EBCDIC conversion by looking for non-printable characters and issue
> a warning.
> 
> The current solution deliberately makes the following tradeoffs:
> 
> * Do not make this a hard failure, there may be a very small number
>   of users who put characters with diacritics on their EBCDIC-encoded
>   command lines.
> * Do not use heuristics with arbitrary thresholds, these may fail
>   intermittently on, e.g., punctuation-heavy command lines, and having
>   to tune thresholds in subsequent patches is not desirable.
> * Use the invariant subset of EBCDIC to determine whether characters
>   are printable, the vast majority of command lines will contain
>   characters from this subset.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828140057.1728173-1-iii@linux.ibm.com?part=2


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC
  2026-08-28 14:09   ` sashiko-bot
@ 2026-08-31 12:14     ` Ilya Leoshkevich
  2026-08-31 13:37       ` Heiko Carstens
  0 siblings, 1 reply; 7+ messages in thread
From: Ilya Leoshkevich @ 2026-08-31 12:14 UTC (permalink / raw)
  To: sashiko-reviews
  Cc: Christian Borntraeger, linux-s390, Vasily Gorbik,
	Alexander Gordeev, Heiko Carstens



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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3 1/2] s390/ebcdic: Add character classes for the invariant subset of EBCDIC
  2026-08-31 12:14     ` Ilya Leoshkevich
@ 2026-08-31 13:37       ` Heiko Carstens
  0 siblings, 0 replies; 7+ messages in thread
From: Heiko Carstens @ 2026-08-31 13:37 UTC (permalink / raw)
  To: Ilya Leoshkevich
  Cc: sashiko-reviews, Christian Borntraeger, linux-s390, Vasily Gorbik,
	Alexander Gordeev

On Mon, Aug 31, 2026 at 02:14:19PM +0200, Ilya Leoshkevich wrote:
> > > +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)?

I guess this is indeed wrong, and 0x4a should be mapped to "0"?

> > > +	_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.
> 
> 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.

For 0xc0 I don't see a problem - it matches what we have in lib/ctype.c, which
also maps "{" to _P.

That only leaves the question above about 0x4a.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-31 13:37 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2026-08-31 12:14     ` Ilya Leoshkevich
2026-08-31 13:37       ` Heiko Carstens
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox