linux-kernel - Re: [PATCH RFC 2/2] wcslen() prototype in string.h

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250325214516.GA672870@ax162>
Date: Tue, 25 Mar 2025 14:45:16 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Andy Shevchenko <andy@...nel.org>
Cc: Kees Cook <kees@...nel.org>,
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>,
	Ard Biesheuvel <ardb@...nel.org>, linux-kernel@...r.kernel.org,
	linux-hardening@...r.kernel.org, llvm@...ts.linux.dev,
	linux-efi@...r.kernel.org
Subject: Re: [PATCH RFC 2/2] wcslen() prototype in string.h

On Tue, Mar 25, 2025 at 07:05:28PM +0200, Andy Shevchenko wrote:
> On Tue, Mar 25, 2025 at 09:58:47AM -0700, Nathan Chancellor wrote:
> > On Tue, Mar 25, 2025 at 06:17:34PM +0200, Andy Shevchenko wrote:
> > > On Tue, Mar 25, 2025 at 08:45:19AM -0700, Nathan Chancellor wrote:
> 
> ...
> 
> > > > Rename the efi function to avoid the conflict.
> > > 
> > > Hmm... Why not split this to two, rename patch as a standalone makes sense to
> > > me even outside of this series.
> > 
> > How so? If nls.h is not included in printk.c via string.h, which does
> > not happen without this series, what value does the rename have? I do
> > not mind splitting it up that way to keep things cleaner, I am just
> > wondering what would be the justification in the changelog (I guess just
> > that nls.h may get included in the future for some reason)?
> 
> Inside this series the justification is obvious (a.k.a. the same), outside
> yes something like "Put EFI specific function to the respective namespace
> to avoid potential clash in the future when including another header."

Okay, sounds reasonable to me. This is what I ended up with for that
change, which will become patch one of the series.

>From f79267c05d2853c034e2c490f171aed064b85dd6 Mon Sep 17 00:00:00 2001
From: Nathan Chancellor <nathan@...nel.org>
Date: Tue, 25 Mar 2025 14:31:25 -0700
Subject: [PATCH] efi: libstub: Rename utf8_to_utf32() to avoid collision with
 NLS version

A future change will add nls.h to string.h, which will break the libstub
build because NLS includes its own version of utf8_to_utf32():

  drivers/firmware/efi/libstub/printk.c:27:5: error: static declaration of 'utf8_to_utf32' follows non-static declaration
     27 | u32 utf8_to_utf32(const u8 **s8)
        |     ^
  include/linux/nls.h:55:12: note: previous declaration is here
     55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
        |            ^
  drivers/firmware/efi/libstub/printk.c:85:26: error: too few arguments to function call, expected 3, have 1
     85 |                 c32 = utf8_to_utf32(&s8);
        |                       ~~~~~~~~~~~~~    ^
  include/linux/nls.h:55:12: note: 'utf8_to_utf32' declared here
     55 | extern int utf8_to_utf32(const u8 *s, int len, unicode_t *pu);
        |            ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  2 errors generated.

Rename the libstub version since it is static to avoid the conflict.

Signed-off-by: Nathan Chancellor <nathan@...nel.org>
---
 drivers/firmware/efi/libstub/printk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/efi/libstub/printk.c b/drivers/firmware/efi/libstub/printk.c
index 3a67a2cea7bd..334f7e89845c 100644
--- a/drivers/firmware/efi/libstub/printk.c
+++ b/drivers/firmware/efi/libstub/printk.c
@@ -24,7 +24,7 @@ void efi_char16_puts(efi_char16_t *str)
 }
 
 static
-u32 utf8_to_utf32(const u8 **s8)
+u32 efi_utf8_to_utf32(const u8 **s8)
 {
 	u32 c32;
 	u8 c0, cx;
@@ -82,7 +82,7 @@ void efi_puts(const char *str)
 	while (*s8) {
 		if (*s8 == '\n')
 			buf[pos++] = L'\r';
-		c32 = utf8_to_utf32(&s8);
+		c32 = efi_utf8_to_utf32(&s8);
 		if (c32 < 0x10000) {
 			/* Characters in plane 0 use a single word. */
 			buf[pos++] = c32;
-- 
2.49.0

Then the first patch (which becomes the second) becomes:

diff --git a/include/linux/string.h b/include/linux/string.h
index 0403a4ca4c11..45e01cf3434c 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -7,6 +7,7 @@
 #include <linux/cleanup.h>	/* for DEFINE_FREE() */
 #include <linux/compiler.h>	/* for inline */
 #include <linux/types.h>	/* for size_t */
+#include <linux/nls.h>		/* for wchar_t */
 #include <linux/stddef.h>	/* for NULL */
 #include <linux/err.h>		/* for ERR_PTR() */
 #include <linux/errno.h>	/* for E2BIG */
@@ -203,6 +204,7 @@ extern __kernel_size_t strlen(const char *);
 #ifndef __HAVE_ARCH_STRNLEN
 extern __kernel_size_t strnlen(const char *,__kernel_size_t);
 #endif
+extern __kernel_size_t wcslen(const wchar_t *s);
 #ifndef __HAVE_ARCH_STRPBRK
 extern char * strpbrk(const char *,const char *);
 #endif
diff --git a/lib/string.c b/lib/string.c
index eb4486ed40d2..1aa09925254b 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -21,6 +21,7 @@
 #include <linux/errno.h>
 #include <linux/limits.h>
 #include <linux/linkage.h>
+#include <linux/nls.h>
 #include <linux/stddef.h>
 #include <linux/string.h>
 #include <linux/types.h>
@@ -429,6 +430,16 @@ size_t strnlen(const char *s, size_t count)
 EXPORT_SYMBOL(strnlen);
 #endif
 
+size_t wcslen(const wchar_t *s)
+{
+	const wchar_t *sc;
+
+	for (sc = s; *sc != '\0'; ++sc)
+		/* nothing */;
+	return sc - s;
+}
+EXPORT_SYMBOL(wcslen);
+
 #ifndef __HAVE_ARCH_STRSPN
 /**
  * strspn - Calculate the length of the initial substring of @s which only contain letters in @accept
---

with basically the same changelog.

I will send v2 either tomorrow afternoon or Thursday morning to give a
little more time for initial review/comments.

Thanks for the quick feedback!

Cheers,
Nathan