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

[David Laight <david.laight.linux@gmail.com>]

On Thu, 27 Mar 2025 23:34:29 +0100
Ben Hutchings <ben@decadent.org.uk> wrote:

> 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)))

Given that, for 7-bit ascii, isprint(uc) is equivalent to (uc >= 0x20 && uc <= 0x7e)
you can do:
	if (uc >= 0x20 && uc <= 0x7e)
		c = uc;
	else
		c = uc ? '!' : 0;

  David

> 		c = uc;
> 	else
> 		c = '!';
> 
> Ben.
>