[PATCH 0/2] lib/ucs2_string.c: fix out-of-bounds read in ucs2_strnlen()

[PATCH 0/2] lib/ucs2_string.c: fix out-of-bounds read in ucs2_strnlen()

[Vincent Mailhol]

This series fixes an off-by-one out-of-bounds read in ucs2_strnlen().

The first patch is a simple refactor which fixes the code indentation.
The second and last patch is the real fix.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Vincent Mailhol (2):
      lib/ucs2_string.c: fix indentation
      lib/ucs2_string.c: fix out-of-bounds read in ucs2_strnlen()

 lib/ucs2_string.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)
---
base-commit: 1590cf0329716306e948a8fc29f1d3ee87d3989f
change-id: 20260722-fix-ucs2_strnlen-145f5d4b1f6b

Best regards,
-- 
Vincent Mailhol <mailhol@kernel.org>

[PATCH 1/2] lib/ucs2_string.c: fix indentation

[Vincent Mailhol]

checkpatch.pl reports a total of 19 incorrect use of spaces instead of
tabulations in lib/ucs2_string.c.

Apply

  ./scripts/checkpatch.pl --fix-inplace lib/ucs2_string.c

to fix them all.

Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
 lib/ucs2_string.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/lib/ucs2_string.c b/lib/ucs2_string.c
index dfb4f2358cab..f07fcdad764b 100644
--- a/lib/ucs2_string.c
+++ b/lib/ucs2_string.c
@@ -6,18 +6,18 @@
 unsigned long
 ucs2_strnlen(const ucs2_char_t *s, size_t maxlength)
 {
-        unsigned long length = 0;
+	unsigned long length = 0;
 
-        while (*s++ != 0 && length < maxlength)
-                length++;
-        return length;
+	while (*s++ != 0 && length < maxlength)
+		length++;
+	return length;
 }
 EXPORT_SYMBOL(ucs2_strnlen);
 
 unsigned long
 ucs2_strlen(const ucs2_char_t *s)
 {
-        return ucs2_strnlen(s, ~0UL);
+	return ucs2_strnlen(s, ~0UL);
 }
 EXPORT_SYMBOL(ucs2_strlen);
 
@@ -28,7 +28,7 @@ EXPORT_SYMBOL(ucs2_strlen);
 unsigned long
 ucs2_strsize(const ucs2_char_t *data, unsigned long maxlength)
 {
-        return ucs2_strnlen(data, maxlength/sizeof(ucs2_char_t)) * sizeof(ucs2_char_t);
+	return ucs2_strnlen(data, maxlength/sizeof(ucs2_char_t)) * sizeof(ucs2_char_t);
 }
 EXPORT_SYMBOL(ucs2_strsize);
 
@@ -87,19 +87,19 @@ EXPORT_SYMBOL(ucs2_strscpy);
 int
 ucs2_strncmp(const ucs2_char_t *a, const ucs2_char_t *b, size_t len)
 {
-        while (1) {
-                if (len == 0)
-                        return 0;
-                if (*a < *b)
-                        return -1;
-                if (*a > *b)
-                        return 1;
-                if (*a == 0) /* implies *b == 0 */
-                        return 0;
-                a++;
-                b++;
-                len--;
-        }
+	while (1) {
+		if (len == 0)
+			return 0;
+		if (*a < *b)
+			return -1;
+		if (*a > *b)
+			return 1;
+		if (*a == 0) /* implies *b == 0 */
+			return 0;
+		a++;
+		b++;
+		len--;
+	}
 }
 EXPORT_SYMBOL(ucs2_strncmp);
 

-- 
2.54.0

[PATCH 2/2] lib/ucs2_string.c: fix out-of-bounds read in ucs2_strnlen()

[Vincent Mailhol]

ucs2_strnlen() checks the current character before checking whether the
caller-provided maximum length has been reached. If the input is not
NUL-terminated within that bound, the loop can read one ucs2_char_t past
the limit.

Test the length before dereferencing to prevent an off-by-one
out-of-bounds read.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Concerning the Fixes: tag, the ucs2_strnlen() function can be traced as
follow:

  - commit 0635eb8a54cf ("Move utf16 functions to kernel core and
    rename") renames it from utf16_strnlen() to ucs2_strnlen() as we
    know it today.

  - commit a2940908391f ("efivars: String functions") renames
    utf8_strlen() to utf16_strnlen().

  - utf8_strlen() is present since the begining of the git history in
    commit 1da177e4c3f4 ("Linux-2.6.12-rc2")

And that of-by-one issue was present since the beginning and survived
all the renamings. This is why I picked 1da177e4c3f4 in the Fixes tag.
---
 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 f07fcdad764b..1f7dd4eb640a 100644
--- a/lib/ucs2_string.c
+++ b/lib/ucs2_string.c
@@ -8,7 +8,7 @@ ucs2_strnlen(const ucs2_char_t *s, size_t maxlength)
 {
 	unsigned long length = 0;
 
-	while (*s++ != 0 && length < maxlength)
+	while (length < maxlength && *s++ != 0)
 		length++;
 	return length;
 }

-- 
2.54.0

Re: [PATCH 2/2] lib/ucs2_string.c: fix out-of-bounds read in ucs2_strnlen()

[Andrew Morton]

On Wed, 22 Jul 2026 23:04:53 +0200 Vincent Mailhol <mailhol@kernel.org> wrote:

> ucs2_strnlen() checks the current character before checking whether the
> caller-provided maximum length has been reached. If the input is not
> NUL-terminated within that bound, the loop can read one ucs2_char_t past
> the limit.

Is there a known way of hitting this from userspace?  If so, and if the
effect is at all serious then we should prepare a backportable fix
against current mainline - that whitespace tweaking patch can come
later.

Also, AI review points at a few possible pre-existing problems in there
which might interest you:
	https://sashiko.dev/#/patchset/20260722-fix-ucs2_strnlen-v1-0-ba5bade2b026@kernel.org

Thanks.

Re: [PATCH 2/2] lib/ucs2_string.c: fix out-of-bounds read in ucs2_strnlen()

[Vincent Mailhol]

On Thu. 23 Jul. 2026 at 01:01, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 22 Jul 2026 23:04:53 +0200 Vincent Mailhol <mailhol@kernel.org> wrote:
>
> > ucs2_strnlen() checks the current character before checking whether the
> > caller-provided maximum length has been reached. If the input is not
> > NUL-terminated within that bound, the loop can read one ucs2_char_t past
> > the limit.
>
> Is there a known way of hitting this from userspace?

I don't see an easy way to exploit this weakness. For an actual kernel
oops to occur, the buffer would need to be at the very end of a page
so that the off-by-one read crosses a page boundary and faults. I
cannot tell whether this is a plausible scenario.

The next worst scenario I can think of would be some kind of timing
attack on CPU branch prediction to leak whether the first ucs2_char_t
past the limit is NUL. That does not seem to provide much leverage in
practice.

So the remaining visible impact would most likely be a KASAN warning.
Although not tested, it seems that admins can trigger this at will by
writing to EFI variables. For normal users, I could not find an easy
path.

> If so, and if the
> effect is at all serious then we should prepare a backportable fix
> against current mainline - that whitespace tweaking patch can come
> later.

Regardless of the above, I agree that it is safer to backport. I will
reorder the series, with the fix first and the tabulation cleanup
next, and add a Cc: stable tag.

> Also, AI review points at a few possible pre-existing problems in there
> which might interest you:
>         https://sashiko.dev/#/patchset/20260722-fix-ucs2_strnlen-v1-0-ba5bade2b026@kernel.org

None of these seem to be addressable by some trivial one-liner fixes
and I already have my TODO list. Sorry but I do not plan to invest
more time investigating those other issues.


Yours sincerely,
Vincent Mailhol