Re: [PATCH V3] block: fix conversion of GPT partition name to 7-bit

[Ben Hutchings <ben@decadent.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 2214 bytes --]

On Wed, 2025-03-05 at 10:21 +0800, Ming Lei wrote:
> From: Olivier Gayot <olivier.gayot@canonical.com>
> 
> The utf16_le_to_7bit function claims to, naively, convert a UTF-16
> string to a 7-bit ASCII string. By naively, we mean that it:
>  * drops the first byte of every character in the original UTF-16 string
>  * checks if all characters are printable, and otherwise replaces them
>    by exclamation mark "!".
> 
> This means that theoretically, all characters outside the 7-bit ASCII
> range should be replaced by another character. Examples:
> 
>  * lower-case alpha (ɒ) 0x0252 becomes 0x52 (R)
>  * ligature OE (œ) 0x0153 becomes 0x53 (S)
>  * hangul letter pieup (ㅂ) 0x3142 becomes 0x42 (B)
>  * upper-case gamma (Ɣ) 0x0194 becomes 0x94 (not printable) so gets
>    replaced by "!"

Also any character with low 8 bits equal to 0 terminates the string.

> The result of this conversion for the GPT partition name is passed to
> user-space as PARTNAME via udev, which is confusing and feels questionable.

Indeed.  But this change seems to make it worse!

[...]
> This results in many values which should be replaced by "!" to be kept
> as-is, despite not being valid 7-bit ASCII. Examples:
> 
>  * e with acute accent (é) 0x00E9 becomes 0xE9 - kept as-is because
>    isprint(0xE9) returns 1.
>
>  * euro sign (€) 0x20AC becomes 0xAC - kept as-is because isprint(0xAC)
>    returns 1.
[...]
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -682,7 +682,7 @@ static void utf16_le_to_7bit(const __le16 *in, unsigned int size, u8 *out)
>  	out[size] = 0;
>  
>  	while (i < size) {
> -		u8 c = le16_to_cpu(in[i]) & 0xff;
> +		u8 c = le16_to_cpu(in[i]) & 0x7f;
>  
>  		if (c && !isprint(c))
>  			c = '!';

Now we map 'é' to 'i' and '€' to ','.  Didn't we want to map them to
'!'?

We shouldn't mask the input character; instead we should do a range
check before calling isprint().  Something like:

	u16 uc = le16_to_cpu(in[i]);
	u8 c;

	if (uc < 0x80 && (uc == 0 || isprint(uc)))
		c = uc;
	else
		c = '!';

Ben.

-- 
Ben Hutchings
If God had intended Man to program,
we'd have been born with serial I/O ports.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]