[PATCH 0/7] efi/libstub: Avoid UTF-16 conversion busywork

[PATCH 0/7] efi/libstub: Avoid UTF-16 conversion busywork

[Ard Biesheuvel]

The EFI libstub performs some redundant conversions between UTF-16 and
UTF-8 and back again, which includes dealing with surrogate pairs, which
UEFI implementations themselves simply ignore.

So drop all the homegrown code, and use the existing UCS-2 (== UTF-16
without surrogate pairs) library code where conversion to UTF-8 is
actually needed (the kernel command line).

The remaining handling involves the EFI console, which supports wide
characters natively, so just use those directly.

Cc: Vincent Mailhol <mailhol@kernel.org>
Cc: x86@kernel.org

Ard Biesheuvel (7):
  x86/boot: Drop pointless re-implementation of panic()
  lib/ucs2_string: Avoid WARN in library code
  efi/libstub: Use ucs2_string library for UTF-16 to UTF-8 conversion
  efi/libstub: Avoid efi_puts() for compile time constant strings
  efi/libstub: Output UTF-16 directly from vsnprintf()
  efi/libstub: Add support for printing human readable GUIDs
  efi/libstub: Add efi_snprintf() to construct wide strings

 arch/x86/boot/compressed/error.c               |  19 ---
 arch/x86/boot/compressed/error.h               |   1 -
 arch/x86/boot/compressed/mem.c                 |   2 +-
 drivers/firmware/efi/libstub/Makefile          |   3 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c |  59 +++-----
 drivers/firmware/efi/libstub/efistub.h         |   6 +-
 drivers/firmware/efi/libstub/gop.c             |   6 +-
 drivers/firmware/efi/libstub/printk.c          |  93 ++----------
 drivers/firmware/efi/libstub/vsprintf.c        | 148 +++++++-------------
 lib/ucs2_string.c                              |   2 +-
 10 files changed, 87 insertions(+), 252 deletions(-)

-- 
2.47.3

[PATCH 1/7] x86/boot: Drop pointless re-implementation of panic()

[Ard Biesheuvel]

The decompressor has its own implementation of panic(), which is based
on the vsnprintf() routine provided by the EFI stub.

Relying on the EFI stub from code that does not execute in the context
of the EFI boot services is a bad idea. It is also completely pointless
in this case, given that the only user of this version of panic() only
passes a compile time constant string, without any printf conversions.

So use error() instead of panic() in that case, and drop the panic()
implementation entirely. This is needed so that the EFI stub's
vsnprintf() can be modified in a manner that is incompatible with the
expectations of this caller.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/boot/compressed/error.c | 19 -------------------
 arch/x86/boot/compressed/error.h |  1 -
 arch/x86/boot/compressed/mem.c   |  2 +-
 3 files changed, 1 insertion(+), 21 deletions(-)

diff --git a/arch/x86/boot/compressed/error.c b/arch/x86/boot/compressed/error.c
index 19a8251de506..ce5ed7d8265e 100644
--- a/arch/x86/boot/compressed/error.c
+++ b/arch/x86/boot/compressed/error.c
@@ -22,22 +22,3 @@ void error(char *m)
 	while (1)
 		asm("hlt");
 }
-
-/* EFI libstub  provides vsnprintf() */
-#ifdef CONFIG_EFI_STUB
-void panic(const char *fmt, ...)
-{
-	static char buf[1024];
-	va_list args;
-	int len;
-
-	va_start(args, fmt);
-	len = vsnprintf(buf, sizeof(buf), fmt, args);
-	va_end(args);
-
-	if (len && buf[len - 1] == '\n')
-		buf[len - 1] = '\0';
-
-	error(buf);
-}
-#endif
diff --git a/arch/x86/boot/compressed/error.h b/arch/x86/boot/compressed/error.h
index 31f9e080d61a..87062dea9a20 100644
--- a/arch/x86/boot/compressed/error.h
+++ b/arch/x86/boot/compressed/error.h
@@ -6,6 +6,5 @@
 
 void warn(const char *m);
 void error(char *m) __noreturn;
-void panic(const char *fmt, ...) __noreturn __cold;
 
 #endif /* BOOT_COMPRESSED_ERROR_H */
diff --git a/arch/x86/boot/compressed/mem.c b/arch/x86/boot/compressed/mem.c
index 0e9f84ab4bdc..e1c017b55184 100644
--- a/arch/x86/boot/compressed/mem.c
+++ b/arch/x86/boot/compressed/mem.c
@@ -37,7 +37,7 @@ void arch_accept_memory(phys_addr_t start, phys_addr_t end)
 	/* Platform-specific memory-acceptance call goes here */
 	if (early_is_tdx_guest()) {
 		if (!tdx_accept_memory(start, end))
-			panic("TDX: Failed to accept memory\n");
+			error("TDX: Failed to accept memory\n");
 	} else if (early_is_sevsnp_guest()) {
 		snp_accept_memory(start, end);
 	} else {
-- 
2.47.3

[PATCH 2/7] lib/ucs2_string: Avoid WARN in library code

[Ard Biesheuvel]

It's not really the job of library code to WARN and potentially bring
down the system (with panic_on_warn=1) on a condition that is really
not something to obsess over.

So drop the WARN_ON_ONCE() from ucs2_strscpy().

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 lib/ucs2_string.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
index 1f7dd4eb640a..d391c9d8eba5 100644
--- a/lib/ucs2_string.c
+++ b/lib/ucs2_string.c
@@ -57,7 +57,7 @@ ssize_t ucs2_strscpy(ucs2_char_t *dst, const ucs2_char_t *src, size_t count)
 	 * Ensure that we have a valid amount of space. We need to store at
 	 * least one NUL-character.
 	 */
-	if (count == 0 || WARN_ON_ONCE(count > INT_MAX / sizeof(*dst)))
+	if (count == 0 || count > INT_MAX / sizeof(*dst))
 		return -E2BIG;
 
 	/*
-- 
2.47.3

[PATCH 3/7] efi/libstub: Use ucs2_string library for UTF-16 to UTF-8 conversion

[Ard Biesheuvel]

Don't rely on sprintf() with a wide string conversion modifier to
convert the command line from UTF-16 to UTF-8. Instead, use the
existing ucs2 string library routine that does the same. Note that while
UEFI claims support for UTF-16, in practice it ignores surrogate pairs
entirely, and so the simplified UCS-2 character set (where each
character takes up exactly 2 bytes) is sufficient here.

This removes the only user of sprintf() in the EFI stub, so drop that
function as well.

While at it, make cmdline_addr a char* and remove the pointless casts.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/Makefile          |  3 +-
 drivers/firmware/efi/libstub/efi-stub-helper.c | 59 ++++++--------------
 drivers/firmware/efi/libstub/vsprintf.c        | 11 ----
 3 files changed, 18 insertions(+), 55 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index 77a2b2d74f3f..12c0c7deb5cb 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -66,7 +66,8 @@ KBUILD_AFLAGS			:= $(KBUILD_CFLAGS) -D__ASSEMBLY__
 lib-y				:= efi-stub-helper.o gop.o secureboot.o tpm.o \
 				   file.o mem.o random.o randomalloc.o pci.o \
 				   skip_spaces.o lib-cmdline.o lib-ctype.o \
-				   alignedmem.o printk.o vsprintf.o
+				   alignedmem.o printk.o vsprintf.o \
+				   lib-ucs2_string.o
 
 # include the stub's libfdt dependencies from lib/ when needed
 libfdt-deps			:= fdt_rw.c fdt_ro.c fdt_wip.c fdt.c \
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index f27f2e1f0019..c221d67bfff6 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -12,6 +12,7 @@
 #include <linux/efi.h>
 #include <linux/kernel.h>
 #include <linux/overflow.h>
+#include <linux/ucs2_string.h>
 #include <asm/efi.h>
 #include <asm/setup.h>
 
@@ -335,10 +336,10 @@ char *efi_convert_cmdline(efi_loaded_image_t *image)
 	const efi_char16_t *options = efi_table_attr(image, load_options);
 	u32 options_size = efi_table_attr(image, load_options_size);
 	int options_bytes = 0, safe_options_bytes = 0;  /* UTF-8 bytes */
-	unsigned long cmdline_addr = 0;
 	const efi_char16_t *s2;
 	bool in_quote = false;
 	efi_status_t status;
+	char *cmdline_addr;
 	u32 options_chars;
 
 	if (options_size > 0)
@@ -351,45 +352,18 @@ char *efi_convert_cmdline(efi_loaded_image_t *image)
 	if (options) {
 		s2 = options;
 		while (options_bytes < COMMAND_LINE_SIZE && options_chars--) {
-			efi_char16_t c = *s2++;
-
-			if (c < 0x80) {
-				if (c == L'\0' || c == L'\n')
-					break;
-				if (c == L'"')
-					in_quote = !in_quote;
-				else if (!in_quote && isspace((char)c))
-					safe_options_bytes = options_bytes;
-
-				options_bytes++;
-				continue;
-			}
-
-			/*
-			 * Get the number of UTF-8 bytes corresponding to a
-			 * UTF-16 character.
-			 * The first part handles everything in the BMP.
-			 */
-			options_bytes += 2 + (c >= 0x800);
-			/*
-			 * Add one more byte for valid surrogate pairs. Invalid
-			 * surrogates will be replaced with 0xfffd and take up
-			 * only 3 bytes.
-			 */
-			if ((c & 0xfc00) == 0xd800) {
-				/*
-				 * If the very last word is a high surrogate,
-				 * we must ignore it since we can't access the
-				 * low surrogate.
-				 */
-				if (!options_chars) {
-					options_bytes -= 3;
-				} else if ((*s2 & 0xfc00) == 0xdc00) {
-					options_bytes++;
-					options_chars--;
-					s2++;
-				}
-			}
+			efi_char16_t c[2] = { *s2++, L'\0' };
+
+			if (c[0] == L'\0' || c[0] == L'\n')
+				break;
+
+			// Check whether the current position is a safe
+			// truncation point
+			in_quote ^= (c[0] == L'"');
+			if (!in_quote && isspace((char)c[0]))
+				safe_options_bytes = options_bytes;
+
+			options_bytes += ucs2_utf8size(c);
 		}
 		if (options_bytes >= COMMAND_LINE_SIZE) {
 			options_bytes = safe_options_bytes;
@@ -405,10 +379,9 @@ char *efi_convert_cmdline(efi_loaded_image_t *image)
 	if (status != EFI_SUCCESS)
 		return NULL;
 
-	snprintf((char *)cmdline_addr, options_bytes, "%.*ls",
-		 options_bytes - 1, options);
+	ucs2_as_utf8(cmdline_addr, options, options_bytes);
 
-	return (char *)cmdline_addr;
+	return cmdline_addr;
 }
 
 /**
diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
index 71c71c222346..dba136679172 100644
--- a/drivers/firmware/efi/libstub/vsprintf.c
+++ b/drivers/firmware/efi/libstub/vsprintf.c
@@ -551,14 +551,3 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
 
 	return pos;
 }
-
-int snprintf(char *buf, size_t size, const char *fmt, ...)
-{
-	va_list args;
-	int i;
-
-	va_start(args, fmt);
-	i = vsnprintf(buf, size, fmt, args);
-	va_end(args);
-	return i;
-}
-- 
2.47.3

[PATCH 4/7] efi/libstub: Avoid efi_puts() for compile time constant strings

[Ard Biesheuvel]

efi_puts() performs a UTF-8 to UTF-16 conversion on its input, as the
EFI console's native character set is UTF-16.

This is pointless for compile time constant strings, since we can
simply define those as UTF-16 to begin with.

Note that efi_puts() also performs LF to CR-LF conversion, so this needs
to be taken into account as well.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/gop.c    | 6 +++---
 drivers/firmware/efi/libstub/printk.c | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/libstub/gop.c b/drivers/firmware/efi/libstub/gop.c
index 80dc8cfeb33e..6919e28ba92b 100644
--- a/drivers/firmware/efi/libstub/gop.c
+++ b/drivers/firmware/efi/libstub/gop.c
@@ -309,12 +309,12 @@ static u32 choose_mode_list(efi_graphics_output_protocol_t *gop)
 	efi_status_t status;
 
 	efi_printk("Available graphics modes are 0-%u\n", max_mode-1);
-	efi_puts("  * = current mode\n"
-		 "  - = unusable mode\n");
+	efi_char16_puts(L"  * = current mode\r\n"
+			 "  - = unusable mode\r\n");
 
 	choose_mode(gop, match_list, (void *)cur_mode);
 
-	efi_puts("\nPress any key to continue (or wait 10 seconds)\n");
+	efi_char16_puts(L"\r\nPress any key to continue (or wait 10 seconds)\r\n");
 	status = efi_wait_for_key(10 * EFI_USEC_PER_SEC, &key);
 	if (status != EFI_SUCCESS && status != EFI_TIMEOUT) {
 		efi_err("Unable to read key, continuing in 10 seconds\n");
diff --git a/drivers/firmware/efi/libstub/printk.c b/drivers/firmware/efi/libstub/printk.c
index bc599212c05d..f36639886d00 100644
--- a/drivers/firmware/efi/libstub/printk.c
+++ b/drivers/firmware/efi/libstub/printk.c
@@ -136,7 +136,7 @@ int efi_printk(const char *fmt, ...)
 		return 0;
 
 	if (loglevel >= 0)
-		efi_puts("EFI stub: ");
+		efi_char16_puts(L"EFI stub: ");
 
 	fmt = printk_skip_level(fmt);
 
@@ -146,7 +146,7 @@ int efi_printk(const char *fmt, ...)
 
 	efi_puts(printf_buf);
 	if (printed >= sizeof(printf_buf)) {
-		efi_puts("[Message truncated]\n");
+		efi_char16_puts(L"[Message truncated]\r\n");
 		return -1;
 	}
 
-- 
2.47.3

[PATCH 5/7] efi/libstub: Output UTF-16 directly from vsnprintf()

[Ard Biesheuvel]

The only remaining users of vsnprintf() in the EFI stub are the
diagnostic printk()'s, which are emitted to the console and not recorded
for posterity.

The EFI console uses UTF-16 (or actually, UCS-2) natively, and so all
non-UTF16 strings that are emitted need to be converted. Given the
stub's vsnprintf() support for wide strings (using the %ls conversion
modifier), which uses UTF-16 to UTF-8 conversion internally, the final
conversion to UTF-16 needs to support not just plain ASCII but UTF-8 as
well.

This is all pointless, of course, and it makes more sense to use UTF-16
internally. This removes the need for UTF-16 to UTF-8 conversion in
vsnprintf(), and given that all non-wide string inputs to vsnprintf()
that exist in the stub today are compile time constant ASCII strings,
the need to convert UTF-8 to UTF-16 disappears as well.

So implement efi_vsnprintf() taking a const char *fmt as before, but
outputting a efi_char16_t[] that can be passed to the EFI console
directly, rather than via efi_puts(), leaving the latter unused and
therefore removed.

Note that efi_puts() performs LF to CR-LF conversion internally, so add
this capability to efi_vsnprintf() as well.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h  |  5 +-
 drivers/firmware/efi/libstub/printk.c   | 89 ++----------------
 drivers/firmware/efi/libstub/vsprintf.c | 94 +++-----------------
 3 files changed, 19 insertions(+), 169 deletions(-)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index fd91fc15ec81..36056c624782 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -1078,9 +1078,10 @@ efi_status_t check_platform_features(void);
 
 void *get_efi_config_table(efi_guid_t guid);
 
-/* NOTE: These functions do not print a trailing newline after the string */
 void efi_char16_puts(efi_char16_t *);
-void efi_puts(const char *str);
+
+int efi_vsnprintf(efi_char16_t *buf, size_t size, const char *fmt, va_list ap,
+		  bool crlf);
 
 __printf(1, 2) int efi_printk(char const *fmt, ...);
 
diff --git a/drivers/firmware/efi/libstub/printk.c b/drivers/firmware/efi/libstub/printk.c
index f36639886d00..09476a6d564e 100644
--- a/drivers/firmware/efi/libstub/printk.c
+++ b/drivers/firmware/efi/libstub/printk.c
@@ -23,98 +23,20 @@ void efi_char16_puts(efi_char16_t *str)
 		       output_string, str);
 }
 
-static
-u32 utf8_to_utf32(const u8 **s8)
-{
-	u32 c32;
-	u8 c0, cx;
-	size_t clen, i;
-
-	c0 = cx = *(*s8)++;
-	/*
-	 * The position of the most-significant 0 bit gives us the length of
-	 * a multi-octet encoding.
-	 */
-	for (clen = 0; cx & 0x80; ++clen)
-		cx <<= 1;
-	/*
-	 * If the 0 bit is in position 8, this is a valid single-octet
-	 * encoding. If the 0 bit is in position 7 or positions 1-3, the
-	 * encoding is invalid.
-	 * In either case, we just return the first octet.
-	 */
-	if (clen < 2 || clen > 4)
-		return c0;
-	/* Get the bits from the first octet. */
-	c32 = cx >> clen--;
-	for (i = 0; i < clen; ++i) {
-		/* Trailing octets must have 10 in most significant bits. */
-		cx = (*s8)[i] ^ 0x80;
-		if (cx & 0xc0)
-			return c0;
-		c32 = (c32 << 6) | cx;
-	}
-	/*
-	 * Check for validity:
-	 * - The character must be in the Unicode range.
-	 * - It must not be a surrogate.
-	 * - It must be encoded using the correct number of octets.
-	 */
-	if (c32 > 0x10ffff ||
-	    (c32 & 0xf800) == 0xd800 ||
-	    clen != (c32 >= 0x80) + (c32 >= 0x800) + (c32 >= 0x10000))
-		return c0;
-	*s8 += clen;
-	return c32;
-}
-
-/**
- * efi_puts() - Write a UTF-8 encoded string to the console
- * @str:	UTF-8 encoded string
- */
-void efi_puts(const char *str)
-{
-	efi_char16_t buf[128];
-	size_t pos = 0, lim = ARRAY_SIZE(buf);
-	const u8 *s8 = (const u8 *)str;
-	u32 c32;
-
-	while (*s8) {
-		if (*s8 == '\n')
-			buf[pos++] = L'\r';
-		c32 = utf8_to_utf32(&s8);
-		if (c32 < 0x10000) {
-			/* Characters in plane 0 use a single word. */
-			buf[pos++] = c32;
-		} else {
-			/*
-			 * Characters in other planes encode into a surrogate
-			 * pair.
-			 */
-			buf[pos++] = (0xd800 - (0x10000 >> 10)) + (c32 >> 10);
-			buf[pos++] = 0xdc00 + (c32 & 0x3ff);
-		}
-		if (*s8 == '\0' || pos >= lim - 2) {
-			buf[pos] = L'\0';
-			efi_char16_puts(buf);
-			pos = 0;
-		}
-	}
-}
-
 /**
  * efi_printk() - Print a kernel message
  * @fmt:	format string
  *
  * The first letter of the format string is used to determine the logging level
  * of the message. If the level is less then the current EFI logging level, the
- * message is suppressed. The message will be truncated to 255 bytes.
+ * message is suppressed. The message will be truncated to 255 characters
+ * (ignoring surrogates).
  *
  * Return:	number of printed characters
  */
 int efi_printk(const char *fmt, ...)
 {
-	char printf_buf[256];
+	efi_char16_t printf_buf[256];
 	va_list args;
 	int printed;
 	int loglevel = printk_get_level(fmt);
@@ -141,10 +63,11 @@ int efi_printk(const char *fmt, ...)
 	fmt = printk_skip_level(fmt);
 
 	va_start(args, fmt);
-	printed = vsnprintf(printf_buf, sizeof(printf_buf), fmt, args);
+	printed = efi_vsnprintf(printf_buf, ARRAY_SIZE(printf_buf), fmt, args,
+				true);
 	va_end(args);
 
-	efi_puts(printf_buf);
+	efi_char16_puts(printf_buf);
 	if (printed >= sizeof(printf_buf)) {
 		efi_char16_puts(L"[Message truncated]\r\n");
 		return -1;
diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
index dba136679172..9ac6df268105 100644
--- a/drivers/firmware/efi/libstub/vsprintf.c
+++ b/drivers/firmware/efi/libstub/vsprintf.c
@@ -14,10 +14,12 @@
 
 #include <linux/compiler.h>
 #include <linux/ctype.h>
+#include <linux/efi.h>
 #include <linux/kernel.h>
 #include <linux/limits.h>
 #include <linux/string.h>
 #include <linux/types.h>
+#include <linux/ucs2_string.h>
 
 static
 int skip_atoi(const char **s)
@@ -239,58 +241,6 @@ char get_sign(long long *num, int flags)
 	return 0;
 }
 
-static
-size_t utf16s_utf8nlen(const u16 *s16, size_t maxlen)
-{
-	size_t len, clen;
-
-	for (len = 0; len < maxlen && *s16; len += clen) {
-		u16 c0 = *s16++;
-
-		/* First, get the length for a BMP character */
-		clen = 1 + (c0 >= 0x80) + (c0 >= 0x800);
-		if (len + clen > maxlen)
-			break;
-		/*
-		 * If this is a high surrogate, and we're already at maxlen, we
-		 * can't include the character if it's a valid surrogate pair.
-		 * Avoid accessing one extra word just to check if it's valid
-		 * or not.
-		 */
-		if ((c0 & 0xfc00) == 0xd800) {
-			if (len + clen == maxlen)
-				break;
-			if ((*s16 & 0xfc00) == 0xdc00) {
-				++s16;
-				++clen;
-			}
-		}
-	}
-
-	return len;
-}
-
-static
-u32 utf16_to_utf32(const u16 **s16)
-{
-	u16 c0, c1;
-
-	c0 = *(*s16)++;
-	/* not a surrogate */
-	if ((c0 & 0xf800) != 0xd800)
-		return c0;
-	/* invalid: low surrogate instead of high */
-	if (c0 & 0x0400)
-		return 0xfffd;
-	c1 = **s16;
-	/* invalid: missing low surrogate */
-	if ((c1 & 0xfc00) != 0xdc00)
-		return 0xfffd;
-	/* valid surrogate pair */
-	++(*s16);
-	return (0x10000 - (0xd800 << 10) - 0xdc00) + (c0 << 10) + c1;
-}
-
 #define PUTC(c) \
 do {				\
 	if (pos < size)		\
@@ -298,7 +248,8 @@ do {				\
 	++pos;			\
 } while (0);
 
-int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
+int efi_vsnprintf(efi_char16_t *buf, size_t size, const char *fmt, va_list ap,
+		  bool crlf)
 {
 	/* The maximum space required is to print a 64-bit number in octal */
 	char tmp[(sizeof(unsigned long long) * 8 + 2) / 3];
@@ -336,6 +287,8 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
 
 	for (pos = 0; *fmt; ++fmt) {
 		if (*fmt != '%' || *++fmt == '%') {
+			if (crlf && *fmt == '\n')
+				PUTC('\r');
 			PUTC(*fmt);
 			continue;
 		}
@@ -400,7 +353,7 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
 			else if (qualifier == 'l') {
 		wstring:
 				flags |= WIDE;
-				precision = len = utf16s_utf8nlen((const u16 *)s, precision);
+				precision = len = ucs2_strnlen((const u16 *)s, precision);
 				goto output;
 			}
 			precision = len = strnlen(s, precision);
@@ -505,36 +458,9 @@ int vsnprintf(char *buf, size_t size, const char *fmt, va_list ap)
 		if (flags & WIDE) {
 			const u16 *ws = (const u16 *)s;
 
-			while (len-- > 0) {
-				u32 c32 = utf16_to_utf32(&ws);
-				u8 *s8;
-				size_t clen;
-
-				if (c32 < 0x80) {
-					PUTC(c32);
-					continue;
-				}
-
-				/* Number of trailing octets */
-				clen = 1 + (c32 >= 0x800) + (c32 >= 0x10000);
-
-				len -= clen;
-				s8 = (u8 *)&buf[pos];
-
-				/* Avoid writing partial character */
-				PUTC('\0');
-				pos += clen;
-				if (pos >= size)
-					continue;
-
-				/* Set high bits of leading octet */
-				*s8 = (0xf00 >> 1) >> clen;
-				/* Write trailing octets in reverse order */
-				for (s8 += clen; clen; --clen, c32 >>= 6)
-					*s8-- = 0x80 | (c32 & 0x3f);
-				/* Set low bits of leading octet */
-				*s8 |= c32;
-			}
+			if (pos < size)
+				ucs2_strscpy(&buf[pos], ws, min(len, size - pos));
+			pos += len;
 		} else {
 			while (len-- > 0)
 				PUTC(*s++);
-- 
2.47.3

Re: [PATCH 5/7] efi/libstub: Output UTF-16 directly from vsnprintf()

[David Laight]

On Sun,  6 Sep 2026 15:08:23 +0200
Ard Biesheuvel <ardb@kernel.org> wrote:

> The only remaining users of vsnprintf() in the EFI stub are the
> diagnostic printk()'s, which are emitted to the console and not recorded
> for posterity.
> 
> The EFI console uses UTF-16 (or actually, UCS-2) natively, and so all
> non-UTF16 strings that are emitted need to be converted. Given the
> stub's vsnprintf() support for wide strings (using the %ls conversion
> modifier), which uses UTF-16 to UTF-8 conversion internally, the final
> conversion to UTF-16 needs to support not just plain ASCII but UTF-8 as
> well.
> 
> This is all pointless, of course, and it makes more sense to use UTF-16
> internally. This removes the need for UTF-16 to UTF-8 conversion in
> vsnprintf(), and given that all non-wide string inputs to vsnprintf()
> that exist in the stub today are compile time constant ASCII strings,
> the need to convert UTF-8 to UTF-16 disappears as well.
> 
> So implement efi_vsnprintf() taking a const char *fmt as before, but
> outputting a efi_char16_t[] that can be passed to the EFI console
> directly, rather than via efi_puts(), leaving the latter unused and
> therefore removed.
> 
> Note that efi_puts() performs LF to CR-LF conversion internally, so add
> this capability to efi_vsnprintf() as well.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
...
>  /**
>   * efi_printk() - Print a kernel message
>   * @fmt:	format string
>   *
>   * The first letter of the format string is used to determine the logging level
>   * of the message. If the level is less then the current EFI logging level, the
> - * message is suppressed. The message will be truncated to 255 bytes.
> + * message is suppressed. The message will be truncated to 255 characters
> + * (ignoring surrogates).
>   *
>   * Return:	number of printed characters
>   */
>  int efi_printk(const char *fmt, ...)
>  {
> -	char printf_buf[256];
> +	efi_char16_t printf_buf[256];
>  	va_list args;
>  	int printed;
>  	int loglevel = printk_get_level(fmt);
> @@ -141,10 +63,11 @@ int efi_printk(const char *fmt, ...)
>  	fmt = printk_skip_level(fmt);
>  
>  	va_start(args, fmt);
> -	printed = vsnprintf(printf_buf, sizeof(printf_buf), fmt, args);
> +	printed = efi_vsnprintf(printf_buf, ARRAY_SIZE(printf_buf), fmt, args,
> +				true);
>  	va_end(args);
>  
> -	efi_puts(printf_buf);
> +	efi_char16_puts(printf_buf);
>  	if (printed >= sizeof(printf_buf)) {

You missed that sizeof().
Be nice to have a note about the size not being in bytes.

David

[PATCH 6/7] efi/libstub: Add support for printing human readable GUIDs

[Ard Biesheuvel]

Add support for the %pUl printk conversion specifier, which takes a
pointer to a GUID and prints it in the usual format:

  aaaaaaaa-bbbb-cccc-dddd-dddddddddddd

Co-developed-by: Vincent Mailhol <mailhol@kernel.org>
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/vsprintf.c | 40 +++++++++++++++++---
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
index 9ac6df268105..4c11cee6c722 100644
--- a/drivers/firmware/efi/libstub/vsprintf.c
+++ b/drivers/firmware/efi/libstub/vsprintf.c
@@ -111,6 +111,9 @@ char *put_dec(char *end, unsigned long long n)
 	return p;
 }
 
+/* we are called with base 8, 10 or 16, only, thus don't need "G..."  */
+static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
+
 static
 char *number(char *end, unsigned long long num, int base, char locase)
 {
@@ -119,9 +122,6 @@ char *number(char *end, unsigned long long num, int base, char locase)
 	 * produces same digits or (maybe lowercased) letters
 	 */
 
-	/* we are called with base 8, 10 or 16, only, thus don't need "G..."  */
-	static const char digits[16] = "0123456789ABCDEF"; /* "GHIJKLMNOPQRSTUVWXYZ"; */
-
 	switch (base) {
 	case 10:
 		if (num != 0)
@@ -142,6 +142,29 @@ char *number(char *end, unsigned long long num, int base, char locase)
 	return end;
 }
 
+static char *guid_to_str(const efi_guid_t *guid, char *out, char locase)
+{
+	static const u8 guid_index[UUID_SIZE] = {
+		3, 2, 1, 0, 5, 4, 7, 6, 8, 9, 10, 11, 12, 13, 14, 15,
+	};
+
+	for (int i = 0, p = 0; i < ARRAY_SIZE(guid_index); i++) {
+		u8 byte = guid->b[guid_index[i]];
+
+		out[p++] = locase | digits[byte >> 4];
+		out[p++] = locase | digits[byte & 0xf];
+
+		switch (i) {
+		case 3:
+		case 5:
+		case 7:
+		case 9:
+			out[p++] = '-';
+		}
+	}
+	return out;
+}
+
 #define ZEROPAD	1		/* pad with zero */
 #define SIGN	2		/* unsigned/signed long */
 #define PLUS	4		/* show plus */
@@ -251,8 +274,7 @@ do {				\
 int efi_vsnprintf(efi_char16_t *buf, size_t size, const char *fmt, va_list ap,
 		  bool crlf)
 {
-	/* The maximum space required is to print a 64-bit number in octal */
-	char tmp[(sizeof(unsigned long long) * 8 + 2) / 3];
+	char tmp[UUID_STRING_LEN];
 	char *tmp_end = &tmp[ARRAY_SIZE(tmp)];
 	long long num;
 	int base;
@@ -365,6 +387,14 @@ int efi_vsnprintf(efi_char16_t *buf, size_t size, const char *fmt, va_list ap,
 			break;
 
 		case 'p':
+			if (fmt[1] == 'U' && (fmt[2] | 0x20) == 'l') {
+				flags &= LEFT;
+				s = guid_to_str(va_arg(args, efi_guid_t *), tmp, fmt[2] & 0x20);
+				precision = len = UUID_STRING_LEN;
+				fmt += 2;
+				goto output;
+			}
+
 			if (precision < 0)
 				precision = 2 * sizeof(void *);
 			fallthrough;
-- 
2.47.3

[PATCH 7/7] efi/libstub: Add efi_snprintf() to construct wide strings

[Ard Biesheuvel]

The native EFI character set is UTF-16 (or in practice, UCS-2).
Implement efi_snprintf() to construct UTF-16 strings using printf style
templates. This will be used in a subsequent patch to set the
LoaderDevicePartUUID EFI variable.

Link: https://lore.kernel.org/all/20260903-efi_stub_bli-v2-1-dbf7ba915117@kernel.org/
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/efistub.h  |  1 +
 drivers/firmware/efi/libstub/vsprintf.c | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 36056c624782..880c1d0c464b 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -1084,6 +1084,7 @@ int efi_vsnprintf(efi_char16_t *buf, size_t size, const char *fmt, va_list ap,
 		  bool crlf);
 
 __printf(1, 2) int efi_printk(char const *fmt, ...);
+__printf(3, 4) int efi_snprintf(efi_char16_t *buf, size_t size, const char *fmt, ...);
 
 void efi_free(unsigned long size, unsigned long addr);
 DEFINE_FREE(efi_pool, void *, if (_T) efi_bs_call(free_pool, _T));
diff --git a/drivers/firmware/efi/libstub/vsprintf.c b/drivers/firmware/efi/libstub/vsprintf.c
index 4c11cee6c722..0c1000ccfe71 100644
--- a/drivers/firmware/efi/libstub/vsprintf.c
+++ b/drivers/firmware/efi/libstub/vsprintf.c
@@ -507,3 +507,14 @@ int efi_vsnprintf(efi_char16_t *buf, size_t size, const char *fmt, va_list ap,
 
 	return pos;
 }
+
+int efi_snprintf(efi_char16_t *buf, size_t size, const char *fmt, ...)
+{
+       va_list args;
+       int i;
+
+       va_start(args, fmt);
+       i = efi_vsnprintf(buf, size, fmt, args, false);
+       va_end(args);
+       return i;
+}
-- 
2.47.3

Re: [PATCH 0/7] efi/libstub: Avoid UTF-16 conversion busywork

[Vincent Mailhol]

On 06/09/2026 at 15:08, Ard Biesheuvel wrote:
> The EFI libstub performs some redundant conversions between UTF-16 and
> UTF-8 and back again, which includes dealing with surrogate pairs, which
> UEFI implementations themselves simply ignore.
> 
> So drop all the homegrown code, and use the existing UCS-2 (== UTF-16
> without surrogate pairs) library code where conversion to UTF-8 is
> actually needed (the kernel command line).
> 
> The remaining handling involves the EFI console, which supports wide
> characters natively, so just use those directly.
> 
> Cc: Vincent Mailhol <mailhol@kernel.org>
> Cc: x86@kernel.org

With the panic() dependency now resolved, this compiles and run well.
I rebased my BLI series on top of it and could confirm the good
behaviour of efi_snprintf(). I just sent a v3 of my BLI patch.

Link: https://lore.kernel.org/linux-efi/20260906-efi_stub_bli-v3-1-e7dc0d6b8fcd@kernel.org/


That said, sashiko has a few findings which looks relevant to me:

Link: https://sashiko.dev/#/patchset/20260906130817.1151961-9-ardb%40kernel.org

with the two most concerning findings being the correct handling of
ucs2_as_utf8()'s options argument when it is NULL and the correct
NUL-termination of the kernel cmdline.

I didn't try to reproduce those issues, but the report looked legit to
me.


Yours sincerely,
Vincent Mailhol

[PATCH] efi: pass NUL-inclusive sizes to ucs2_as_utf8()

[Vincent Mailhol]

An upcoming change will update ucs2_as_utf8() to expose a strscpy()
style API where the size argument is the destination buffer size,
including space for the final NUL terminator.

Some EFI callers currently pass the exact number of UTF-8 payload bytes
that they expect to copy and add the terminator themselves afterwards.
Extend those sizes to include the final NUL terminator so the upcoming
contract change does not truncate the converted output by one byte.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Hi Ard,

I saw that you pushed on efi-libstub-native-utf16 WIP branch [1] and did
some testing, despite those changes not yet submitted for review.

There is an off-by-one error following your ucs2_as_utf8() code
refactor. This patch prevents the issue. It should be cherry-picked just
before your "efi/libstub: Use ucs2_string library for UTF-16 to UTF-8
conversion" commit.

There is one final off-by-one in "efi/libstub: Use ucs2_string library
for UTF-16 to UTF-8 conversion" itself. This is the fix:

	---8<---
	diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
	index e9b714ca811db..db0bde514f7fd 100644
	--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
	+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
	@@ -379,8 +379,7 @@ char *efi_convert_cmdline(efi_loaded_image_t *image)
	 	if (status != EFI_SUCCESS)
	 		return NULL;

	-	ucs2_as_utf8(cmdline_addr, options, options_bytes - 1);
	-	cmdline_addr[options_bytes - 1] = '\0';
	+	ucs2_as_utf8(cmdline_addr, options, options_bytes);

	 	return cmdline_addr;
	 }
	---8<---

[1] https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=efi-libstub-native-utf16
---
 drivers/firmware/efi/efi.c | 2 +-
 fs/efivarfs/vars.c         | 5 ++---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 6d987d7f97781..221804e7d5390 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -304,7 +304,7 @@ static __init int efivar_ssdt_load(void)
 		}
 
 		limit = min(EFIVAR_SSDT_NAME_MAX, name_size);
-		ucs2_as_utf8(utf8_name, name, limit - 1);
+		ucs2_as_utf8(utf8_name, name, limit);
 		if (strncmp(utf8_name, efivar_ssdt, limit) != 0)
 			continue;
 
diff --git a/fs/efivarfs/vars.c b/fs/efivarfs/vars.c
index 6833c3d24b541..1ddc89e13518f 100644
--- a/fs/efivarfs/vars.c
+++ b/fs/efivarfs/vars.c
@@ -237,7 +237,7 @@ efivar_get_utf8name(const efi_char16_t *name16, efi_guid_t *vendor)
 	if (!name)
 		return NULL;
 
-	ucs2_as_utf8(name, name16, len);
+	ucs2_as_utf8(name, name16, len + 1);
 
 	name[len] = '-';
 
@@ -264,8 +264,7 @@ efivar_validate(efi_guid_t vendor, efi_char16_t *var_name, u8 *data,
 	if (!utf8_name)
 		return false;
 
-	ucs2_as_utf8(utf8_name, var_name, utf8_size);
-	utf8_name[utf8_size] = '\0';
+	ucs2_as_utf8(utf8_name, var_name, utf8_size + 1);
 
 	for (i = 0; variable_validate[i].name[0] != '\0'; i++) {
 		const char *name = variable_validate[i].name;
-- 
2.43.0

Re: [PATCH] efi: pass NUL-inclusive sizes to ucs2_as_utf8()

[Ard Biesheuvel]

(drop x86@ from cc)

Hi Vincent,

On Tue, 8 Sep 2026, at 19:24, Vincent Mailhol wrote:
> An upcoming change will update ucs2_as_utf8() to expose a strscpy()
> style API where the size argument is the destination buffer size,
> including space for the final NUL terminator.
>
> Some EFI callers currently pass the exact number of UTF-8 payload bytes
> that they expect to copy and add the terminator themselves afterwards.
> Extend those sizes to include the final NUL terminator so the upcoming
> contract change does not truncate the converted output by one byte.
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
> ---
> Hi Ard,
>
> I saw that you pushed on efi-libstub-native-utf16 WIP branch [1] and did
> some testing, despite those changes not yet submitted for review.
>


Thanks for the review, but I am going to drop that change. I am also going
to drop the ucs2_strscpy() call from my series, and use memcpy() instead.
The guaranteed NUL terminator is not really needed for a printf() style
function, and it actually interferes with the precision handling.

Instead, I'm adding a ucs2_to_utf8() that takes a separate limit argument,
and putting efi_convert_cmdline() on its head entirely so we don't go
through the input character by character twice.

I'll have v2 out shortly, including your v3 with my tweaks on top.
Please take the time to review on list if you have the bandwidth. Thanks.