mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem()
@ 2026-09-26 20:56 Muhammad Bilal
  2026-09-26 20:56 ` [PATCH 2/2] platform/x86: hp-bioscfg: fix heap OOB read and non-ASCII truncation in hp_get_string_from_buffer() Muhammad Bilal
  2026-09-28  8:11 ` [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Andy Shevchenko
  0 siblings, 2 replies; 7+ messages in thread
From: Muhammad Bilal @ 2026-09-26 20:56 UTC (permalink / raw)
  To: Kees Cook
  Cc: Muhammad Bilal, Andy Shevchenko, Ilpo Järvinen,
	linux-hardening, linux-kernel

ESCAPE_SPECIAL escapes '\\', '\a', '\e' and '"' as one group. A caller
that wants '\\' escaped but needs '"' left untouched, because it has
its own handling for quotes or because quoting is not meaningful in
its output, currently has no way to pull just the backslash case out
of that set.

Add ESCAPE_BACKSLASH, which escapes only '\\'. It can be combined with
ESCAPE_SPACE, or used on its own, without pulling in ESCAPE_SPECIAL's
quote and control-character handling.

This only adds a new opt-in flag; no existing caller changes
behavior.

Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 include/linux/string_helpers.h |  3 ++-
 lib/string_helpers.c           | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/linux/string_helpers.h b/include/linux/string_helpers.h
index 3fb88a1..111114d 100644
--- a/include/linux/string_helpers.h
+++ b/include/linux/string_helpers.h
@@ -72,8 +72,9 @@ static inline int string_unescape_any_inplace(char *buf)
 #define ESCAPE_NA		BIT(6)
 #define ESCAPE_NAP		BIT(7)
 #define ESCAPE_APPEND		BIT(8)
+#define ESCAPE_BACKSLASH	BIT(9)
 
-#define ESCAPE_ALL_MASK		GENMASK(8, 0)
+#define ESCAPE_ALL_MASK		GENMASK(9, 0)
 
 int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
 		unsigned int flags, const char *only);
diff --git a/lib/string_helpers.c b/lib/string_helpers.c
index 98d6ed0..f35234a 100644
--- a/lib/string_helpers.c
+++ b/lib/string_helpers.c
@@ -438,6 +438,24 @@ static bool escape_special(unsigned char c, char **dst, char *end)
 	return true;
 }
 
+static bool escape_backslash(unsigned char c, char **dst, char *end)
+{
+	char *out = *dst;
+
+	if (c != '\\')
+		return false;
+
+	if (out < end)
+		*out = '\\';
+	++out;
+	if (out < end)
+		*out = '\\';
+	++out;
+
+	*dst = out;
+	return true;
+}
+
 static bool escape_null(unsigned char c, char **dst, char *end)
 {
 	char *out = *dst;
@@ -558,6 +576,8 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
  *		escape only non-printable or non-ascii characters
  *	%ESCAPE_APPEND:
  *		append characters from @only to be escaped by the given classes
+ *	%ESCAPE_BACKSLASH:
+ *		'\\' - backslash, without the rest of %ESCAPE_SPECIAL
  *
  * %ESCAPE_APPEND would help to pass additional characters to the escaped, when
  * one of %ESCAPE_NP, %ESCAPE_NA, or %ESCAPE_NAP is provided.
@@ -628,6 +648,9 @@ int string_escape_mem(const char *src, size_t isz, char *dst, size_t osz,
 		if (flags & ESCAPE_SPECIAL && escape_special(c, &p, end))
 			continue;
 
+		if (flags & ESCAPE_BACKSLASH && escape_backslash(c, &p, end))
+			continue;
+
 		if (flags & ESCAPE_NULL && escape_null(c, &p, end))
 			continue;
 
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] platform/x86: hp-bioscfg: fix heap OOB read and non-ASCII truncation in hp_get_string_from_buffer()
  2026-09-26 20:56 [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Muhammad Bilal
@ 2026-09-26 20:56 ` Muhammad Bilal
  2026-09-28 11:32   ` Ilpo Järvinen
  2026-09-28  8:11 ` [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Andy Shevchenko
  1 sibling, 1 reply; 7+ messages in thread
From: Muhammad Bilal @ 2026-09-26 20:56 UTC (permalink / raw)
  To: Jorge Lopez, Hans de Goede, Ilpo Järvinen
  Cc: Muhammad Bilal, platform-driver-x86, linux-kernel, stable

hp_get_string_from_buffer() has three problems.

First, the loop that counts how many characters will need
backslash-escaping uses the same variable as both the accumulator and
the loop bound, so once an escape character is found near the end of
the valid range the loop keeps going past the buffer's true character
count, reading out of bounds.

Second, the bounds check 'if (*buffer_size < src_size)' runs after
src has already stepped over the 2-byte length prefix, so if
*buffer_size equals src_size, reading src_size bytes reads 2 bytes
past the end of the input buffer. The pointer and remaining size are
then advanced using the escape-inflated count rather than the actual
number of bytes consumed, desyncing subsequent property parsers.

Third, found during review of the fix for the first two:
utf16s_to_utf8s() takes src by value, so the real UTF-16-to-UTF-8
conversion it performs never reaches dst. A second loop immediately
overwrites dst by re-walking the same src position and copying each
UTF-16 code unit with a truncating cast (dst[i] = *src), escaping
'\\', '\r', '\n', '\t' and turning '"' into '\''. Any character above
0x7f is truncated to its low byte instead of being encoded, so the
function is ASCII-only in practice. For example, given the
two-character UTF-16 string 'eA' with the first character U+00E9
(e-acute), the function currently writes the two raw bytes 0xe9 0x41
into dst, an invalid, truncated sequence, instead of the three bytes
0xc3 0xa9 0x41, the correct UTF-8 encoding of U+00E9 followed by 'A'.

Fix all three by keeping the true character count separate from the
escape-inflated one, checking *buffer_size against the full prefix
plus string length before reading, and replacing the redundant
conversion with a real one: convert into a fixed-size scratch buffer
with utf16s_to_utf8s(), then escape that UTF-8 into dst with
string_escape_mem(). The scratch buffer is sized to MAX_BUFF_SIZE
rather than to src_size, since src_size comes straight from the WMI
buffer and dst_size is MAX_BUFF_SIZE at every caller in this file.

string_escape_mem() has no flag that escapes only a backslash without
also touching '"', so this depends on a separate patch that adds
ESCAPE_BACKSLASH to string_escape_mem(). ESCAPE_SPACE together with
ESCAPE_BACKSLASH escapes '\\', '\f', '\n', '\r', '\t' and '\v'; '\v'
and '\f' were not escaped before, matching Ilpo's comment that this
was likely an oversight rather than intentional. The existing
substitution of '"' for '\'' is kept as an explicit strreplace()
afterward, since string_escape_mem() has no substitution concept and
switching '"' handling to backslash-escaping was ruled out in review.

string_escape_mem()'s return value can exceed the destination size
when the input does not fit, so the NUL terminator position is
clamped to dst_size - 1 before use; writing dst[<return value>] = 0
unconditionally would reintroduce a heap OOB write of exactly the
kind this patch fixes.

Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
Cc: stable@vger.kernel.org
Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 78 ++++++++------------
 1 file changed, 31 insertions(+), 47 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
index 309634c..82c228c 100644
--- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
+++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
@@ -12,6 +12,7 @@
 #include <linux/kernel.h>
 #include <linux/printk.h>
 #include <linux/string.h>
+#include <linux/string_helpers.h>
 #include <linux/wmi.h>
 #include "bioscfg.h"
 #include "../../firmware_attributes_class.h"
@@ -55,72 +56,55 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
 {
 	u16 *src = (u16 *)*buffer;
 	u16 src_size;
-
-	u16 size;
-	int i;
+	u16 orig_size;
+	char utf8_buf[MAX_BUFF_SIZE];
+	int utf8_len;
 	int conv_dst_size;
+	int escaped_len;
 
 	if (*buffer_size < sizeof(u16))
 		return -EINVAL;
 
-	src_size = *(src++);
-	/* size value in u16 chars */
-	size = src_size / sizeof(u16);
+	src_size = *src;
 
 	/* Ensure there is enough space remaining to read and convert
 	 * the string
 	 */
-	if (*buffer_size < src_size)
+	if (*buffer_size < sizeof(u16) + src_size)
 		return -EINVAL;
 
-	for (i = 0; i < size; i++)
-		if (src[i] == '\\' ||
-		    src[i] == '\r' ||
-		    src[i] == '\n' ||
-		    src[i] == '\t')
-			size++;
+	src++;
+	/* size value in u16 chars */
+	orig_size = src_size / sizeof(u16);
 
 	/*
-	 * Conversion is limited to destination string max number of
-	 * bytes.
+	 * Convert from UTF-16 to UTF-8 into a scratch buffer first, then
+	 * escape the result into dst. utf16s_to_utf8s() is capped at
+	 * sizeof(utf8_buf) regardless of orig_size: orig_size comes
+	 * straight from the WMI buffer, and dst_size is MAX_BUFF_SIZE at
+	 * every caller in this file.
 	 */
-	conv_dst_size = size;
-	if (size >= dst_size)
-		conv_dst_size = dst_size - 1;
+	utf8_len = utf16s_to_utf8s(src, orig_size, UTF16_HOST_ENDIAN,
+				   utf8_buf, sizeof(utf8_buf));
+
+	conv_dst_size = dst_size - 1;
+	escaped_len = string_escape_mem(utf8_buf, utf8_len, dst, conv_dst_size,
+					ESCAPE_SPACE | ESCAPE_BACKSLASH, NULL);
+	if (escaped_len > conv_dst_size)
+		escaped_len = conv_dst_size;
+	dst[escaped_len] = 0;
 
 	/*
-	 * convert from UTF-16 unicode to ASCII
+	 * string_escape_mem() has no equivalent of the original quote
+	 * substitution; keep turning '"' into a plain single quote instead
+	 * of escaping it, to match existing behaviour.
 	 */
-	utf16s_to_utf8s(src, src_size, UTF16_HOST_ENDIAN, dst, conv_dst_size);
-	dst[conv_dst_size] = 0;
-
-	for (i = 0; i < conv_dst_size; i++) {
-		if (*src == '\\' ||
-		    *src == '\r' ||
-		    *src == '\n' ||
-		    *src == '\t') {
-			dst[i++] = '\\';
-			if (i == conv_dst_size)
-				break;
-		}
-
-		if (*src == '\r')
-			dst[i] = 'r';
-		else if (*src == '\n')
-			dst[i] = 'n';
-		else if (*src == '\t')
-			dst[i] = 't';
-		else if (*src == '"')
-			dst[i] = '\'';
-		else
-			dst[i] = *src;
-		src++;
-	}
+	strreplace(dst, '"', '\'');
 
-	*buffer = (u8 *)src;
-	*buffer_size -= size * sizeof(u16);
+	*buffer += sizeof(u16) + src_size;
+	*buffer_size -= sizeof(u16) + src_size;
 
-	return size;
+	return escaped_len;
 }
 
 int hp_get_common_data_from_buffer(u8 **buffer_ptr, u32 *buffer_size,
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem()
  2026-09-26 20:56 [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Muhammad Bilal
  2026-09-26 20:56 ` [PATCH 2/2] platform/x86: hp-bioscfg: fix heap OOB read and non-ASCII truncation in hp_get_string_from_buffer() Muhammad Bilal
@ 2026-09-28  8:11 ` Andy Shevchenko
  2026-09-28  8:17   ` Andy Shevchenko
  2026-09-28 12:59   ` Ilpo Järvinen
  1 sibling, 2 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-09-28  8:11 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: Kees Cook, Andy Shevchenko, Ilpo Järvinen, linux-hardening,
	linux-kernel

On Sat, Sep 26, 2026 at 11:57 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> ESCAPE_SPECIAL escapes '\\', '\a', '\e' and '"' as one group. A caller
> that wants '\\' escaped but needs '"' left untouched, because it has
> its own handling for quotes or because quoting is not meaningful in
> its output, currently has no way to pull just the backslash case out
> of that set.
>
> Add ESCAPE_BACKSLASH, which escapes only '\\'. It can be combined with
> ESCAPE_SPACE, or used on its own, without pulling in ESCAPE_SPECIAL's
> quote and control-character handling.

But why? I believe it can be done in the current implementation using
the last argument @only (id est use the list of the characters you
want to escape).

> This only adds a new opt-in flag; no existing caller changes
> behavior.

No test cases --> automatically NAK.

...

Also you missed printk() update and respective documentation.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem()
  2026-09-28  8:11 ` [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Andy Shevchenko
@ 2026-09-28  8:17   ` Andy Shevchenko
  2026-09-28 12:59   ` Ilpo Järvinen
  1 sibling, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-09-28  8:17 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Muhammad Bilal, Kees Cook, Andy Shevchenko, Ilpo Järvinen,
	linux-hardening, linux-kernel

On Mon, Sep 28, 2026 at 11:11:53AM +0300, Andy Shevchenko wrote:
> On Sat, Sep 26, 2026 at 11:57 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
> >
> > ESCAPE_SPECIAL escapes '\\', '\a', '\e' and '"' as one group. A caller
> > that wants '\\' escaped but needs '"' left untouched, because it has
> > its own handling for quotes or because quoting is not meaningful in
> > its output, currently has no way to pull just the backslash case out
> > of that set.
> >
> > Add ESCAPE_BACKSLASH, which escapes only '\\'. It can be combined with
> > ESCAPE_SPACE, or used on its own, without pulling in ESCAPE_SPECIAL's
> > quote and control-character handling.
> 
> But why? I believe it can be done in the current implementation using
> the last argument @only (id est use the list of the characters you
> want to escape).
> 
> > This only adds a new opt-in flag; no existing caller changes
> > behavior.
> 
> No test cases --> automatically NAK.

...

> Also you missed printk() update and respective documentation.

And on top of that, I have neither cover letter, nor patch 2/2 in my mailbox.
If you think that it's not important to me as lib/string* reviewer/contributor,
you are mistaken (yes, I can retrieve from lore, but this doesn't change
the fact).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] platform/x86: hp-bioscfg: fix heap OOB read and non-ASCII truncation in hp_get_string_from_buffer()
  2026-09-26 20:56 ` [PATCH 2/2] platform/x86: hp-bioscfg: fix heap OOB read and non-ASCII truncation in hp_get_string_from_buffer() Muhammad Bilal
@ 2026-09-28 11:32   ` Ilpo Järvinen
  0 siblings, 0 replies; 7+ messages in thread
From: Ilpo Järvinen @ 2026-09-28 11:32 UTC (permalink / raw)
  To: Muhammad Bilal
  Cc: Jorge Lopez, Hans de Goede, platform-driver-x86, LKML, stable

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

On Sun, 27 Sep 2026, Muhammad Bilal wrote:

> hp_get_string_from_buffer() has three problems.
> 
> First, the loop that counts how many characters will need
> backslash-escaping uses the same variable as both the accumulator and
> the loop bound, so once an escape character is found near the end of
> the valid range the loop keeps going past the buffer's true character
> count, reading out of bounds.
> 
> Second, the bounds check 'if (*buffer_size < src_size)' runs after
> src has already stepped over the 2-byte length prefix, so if
> *buffer_size equals src_size, reading src_size bytes reads 2 bytes
> past the end of the input buffer. The pointer and remaining size are
> then advanced using the escape-inflated count rather than the actual
> number of bytes consumed, desyncing subsequent property parsers.
> 
> Third, found during review of the fix for the first two:

This kind of notes (how the patch came to be) are not content that should 
be put into changelog. Generally (there may be some exceptions), just 
explain what the problem is, not focusing on how it was found. The 
reporter can be credited with Reported-by or Suggested-by tag depending on 
which of them is more suitable for the particular case.

> utf16s_to_utf8s() takes src by value, so the real UTF-16-to-UTF-8
> conversion it performs never reaches dst.

While true from the end result point of view that nothing of the 
intermediate write remains, "never reaches dst" doesn't sound correct 
description of what happens here. 

> A second loop immediately
> overwrites dst by re-walking the same src position and copying each
> UTF-16 code unit with a truncating cast (dst[i] = *src), escaping
> '\\', '\r', '\n', '\t' and turning '"' into '\''. Any character above
> 0x7f is truncated to its low byte instead of being encoded, so the
> function is ASCII-only in practice. For example, given the
> two-character UTF-16 string 'eA' with the first character U+00E9
> (e-acute),

Nit, it's actually possible to include non-ascii characters in the 
changelog. It might be still be useful to retain the textual explanation 
if something manages to mangle the character despite everything but 
normally it just works. (My surname has been mangled countless of times, 
but much less these days. In the early days when git tools were not as 
mature as today, one could even count how many times it got mangled while 
getting passed from person/tool to another :-D).

> the function currently writes the two raw bytes 0xe9 0x41
> into dst, an invalid, truncated sequence, instead of the three bytes
> 0xc3 0xa9 0x41, the correct UTF-8 encoding of U+00E9 followed by 'A'.
> 
> Fix all three by keeping the true character count separate from the
> escape-inflated one, checking *buffer_size against the full prefix
> plus string length before reading, and replacing the redundant
> conversion with a real one: convert into a fixed-size scratch buffer
> with utf16s_to_utf8s(), then escape that UTF-8 into dst with
> string_escape_mem(). The scratch buffer is sized to MAX_BUFF_SIZE
> rather than to src_size, since src_size comes straight from the WMI
> buffer and dst_size is MAX_BUFF_SIZE at every caller in this file.

It's preferrable, if possible, to fix the problems one by one. If it's 
strictly impossible for some reason I cannot see, only then combine what 
has to be combined.

This applies also to cases where something new comes up during a review of 
a patch, if the newly found problem is separate from what the commented 
patch was fixing, it should appear in own patch because it's logically a 
separate problem.

Normally we aim to do logically minimal patches, and as many patches as 
needed in a series to fix all logically disjoint problems one by one.
This allows more focused (and shorter) changelog per patch, and normally 
makes review much simpler than one bigger spaghetti change where it's 
hard for the reviewer to see what relates to which problem.

> string_escape_mem() has no flag that escapes only a backslash without
> also touching '"', so this depends on a separate patch that adds
> ESCAPE_BACKSLASH to string_escape_mem().

Not true anymore because you fixed that in the first patch ;-) so don't 
write stale information into the subsequent changelogs. There's also no 
need to tell something like "feature x was added to y" (in case you're 
tempted to change the wording towards something along those lines).

We work in very much present time when writing a changelog, so if the 
previous patch fixed a problem, we assume in the next patch's changelog 
the problem no longer exists so there's no need to even mention it.

(There may be exceptions in case of very complex, interjoining problems 
but that's not relevant here.)

> ESCAPE_SPACE together with
> ESCAPE_BACKSLASH escapes '\\', '\f', '\n', '\r', '\t' and '\v'; '\v'
> and '\f' were not escaped before, matching Ilpo's comment that this
> was likely an oversight rather than intentional. The existing
> substitution of '"' for '\'' is kept as an explicit strreplace()
> afterward, since string_escape_mem() has no substitution concept and
> switching '"' handling to backslash-escaping was ruled out in review.
> 
> string_escape_mem()'s return value can exceed the destination size
> when the input does not fit, so the NUL terminator position is
> clamped to dst_size - 1 before use; writing dst[<return value>] = 0
> unconditionally would reintroduce a heap OOB write of exactly the
> kind this patch fixes.
> 
> Fixes: a34fc329b189 ("platform/x86: hp-bioscfg: bioscfg")
> Cc: stable@vger.kernel.org
> Suggested-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---

As this series is small, I suggest you send the next version of the entire 
series to all interest parties. Those looking at patch 1 would want to 
know about this change so to understand the use case, therefore make it 
easy to find for the reviewers by including them.

I didn't look much into the diff itself as I believe it gets cleaner once 
you split the change logically into a series of patches.

-- 
 i.

>  drivers/platform/x86/hp/hp-bioscfg/bioscfg.c | 78 ++++++++------------
>  1 file changed, 31 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> index 309634c..82c228c 100644
> --- a/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> +++ b/drivers/platform/x86/hp/hp-bioscfg/bioscfg.c
> @@ -12,6 +12,7 @@
>  #include <linux/kernel.h>
>  #include <linux/printk.h>
>  #include <linux/string.h>
> +#include <linux/string_helpers.h>
>  #include <linux/wmi.h>
>  #include "bioscfg.h"
>  #include "../../firmware_attributes_class.h"
> @@ -55,72 +56,55 @@ int hp_get_string_from_buffer(u8 **buffer, u32 *buffer_size, char *dst, u32 dst_
>  {
>  	u16 *src = (u16 *)*buffer;
>  	u16 src_size;
> -
> -	u16 size;
> -	int i;
> +	u16 orig_size;
> +	char utf8_buf[MAX_BUFF_SIZE];
> +	int utf8_len;
>  	int conv_dst_size;
> +	int escaped_len;
>  
>  	if (*buffer_size < sizeof(u16))
>  		return -EINVAL;
>  
> -	src_size = *(src++);
> -	/* size value in u16 chars */
> -	size = src_size / sizeof(u16);
> +	src_size = *src;
>  
>  	/* Ensure there is enough space remaining to read and convert
>  	 * the string
>  	 */
> -	if (*buffer_size < src_size)
> +	if (*buffer_size < sizeof(u16) + src_size)
>  		return -EINVAL;
>  
> -	for (i = 0; i < size; i++)
> -		if (src[i] == '\\' ||
> -		    src[i] == '\r' ||
> -		    src[i] == '\n' ||
> -		    src[i] == '\t')
> -			size++;
> +	src++;
> +	/* size value in u16 chars */
> +	orig_size = src_size / sizeof(u16);
>  
>  	/*
> -	 * Conversion is limited to destination string max number of
> -	 * bytes.
> +	 * Convert from UTF-16 to UTF-8 into a scratch buffer first, then
> +	 * escape the result into dst. utf16s_to_utf8s() is capped at
> +	 * sizeof(utf8_buf) regardless of orig_size: orig_size comes
> +	 * straight from the WMI buffer, and dst_size is MAX_BUFF_SIZE at
> +	 * every caller in this file.
>  	 */
> -	conv_dst_size = size;
> -	if (size >= dst_size)
> -		conv_dst_size = dst_size - 1;
> +	utf8_len = utf16s_to_utf8s(src, orig_size, UTF16_HOST_ENDIAN,
> +				   utf8_buf, sizeof(utf8_buf));
> +
> +	conv_dst_size = dst_size - 1;
> +	escaped_len = string_escape_mem(utf8_buf, utf8_len, dst, conv_dst_size,
> +					ESCAPE_SPACE | ESCAPE_BACKSLASH, NULL);
> +	if (escaped_len > conv_dst_size)
> +		escaped_len = conv_dst_size;
> +	dst[escaped_len] = 0;
>  
>  	/*
> -	 * convert from UTF-16 unicode to ASCII
> +	 * string_escape_mem() has no equivalent of the original quote
> +	 * substitution; keep turning '"' into a plain single quote instead
> +	 * of escaping it, to match existing behaviour.
>  	 */
> -	utf16s_to_utf8s(src, src_size, UTF16_HOST_ENDIAN, dst, conv_dst_size);
> -	dst[conv_dst_size] = 0;
> -
> -	for (i = 0; i < conv_dst_size; i++) {
> -		if (*src == '\\' ||
> -		    *src == '\r' ||
> -		    *src == '\n' ||
> -		    *src == '\t') {
> -			dst[i++] = '\\';
> -			if (i == conv_dst_size)
> -				break;
> -		}
> -
> -		if (*src == '\r')
> -			dst[i] = 'r';
> -		else if (*src == '\n')
> -			dst[i] = 'n';
> -		else if (*src == '\t')
> -			dst[i] = 't';
> -		else if (*src == '"')
> -			dst[i] = '\'';
> -		else
> -			dst[i] = *src;
> -		src++;
> -	}
> +	strreplace(dst, '"', '\'');
>  
> -	*buffer = (u8 *)src;
> -	*buffer_size -= size * sizeof(u16);
> +	*buffer += sizeof(u16) + src_size;
> +	*buffer_size -= sizeof(u16) + src_size;
>  
> -	return size;
> +	return escaped_len;
>  }
>  
>  int hp_get_common_data_from_buffer(u8 **buffer_ptr, u32 *buffer_size,
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem()
  2026-09-28  8:11 ` [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Andy Shevchenko
  2026-09-28  8:17   ` Andy Shevchenko
@ 2026-09-28 12:59   ` Ilpo Järvinen
  2026-09-28 13:38     ` Andy Shevchenko
  1 sibling, 1 reply; 7+ messages in thread
From: Ilpo Järvinen @ 2026-09-28 12:59 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Muhammad Bilal, Kees Cook, Andy Shevchenko, linux-hardening, LKML

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

On Mon, 28 Sep 2026, Andy Shevchenko wrote:

> On Sat, Sep 26, 2026 at 11:57 PM Muhammad Bilal <meatuni001@gmail.com> wrote:
> >
> > ESCAPE_SPECIAL escapes '\\', '\a', '\e' and '"' as one group. A caller
> > that wants '\\' escaped but needs '"' left untouched, because it has
> > its own handling for quotes or because quoting is not meaningful in
> > its output, currently has no way to pull just the backslash case out
> > of that set.
> >
> > Add ESCAPE_BACKSLASH, which escapes only '\\'. It can be combined with
> > ESCAPE_SPACE, or used on its own, without pulling in ESCAPE_SPECIAL's
> > quote and control-character handling.
> 
> But why? I believe it can be done in the current implementation using
> the last argument @only (id est use the list of the characters you
> want to escape).

Ah, that's my fault for suggesting this and not noticing there was way to 
negatively filter them.

> > This only adds a new opt-in flag; no existing caller changes
> > behavior.
> 
> No test cases --> automatically NAK.
> 
> ...
> 
> Also you missed printk() update and respective documentation.
> 
> 

-- 
 i.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem()
  2026-09-28 12:59   ` Ilpo Järvinen
@ 2026-09-28 13:38     ` Andy Shevchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Andy Shevchenko @ 2026-09-28 13:38 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Muhammad Bilal, Kees Cook, Andy Shevchenko, linux-hardening, LKML

On Mon, Sep 28, 2026 at 3:59 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
> On Mon, 28 Sep 2026, Andy Shevchenko wrote:
> > On Sat, Sep 26, 2026 at 11:57 PM Muhammad Bilal <meatuni001@gmail.com> wrote:

...

> > > Add ESCAPE_BACKSLASH, which escapes only '\\'. It can be combined with
> > > ESCAPE_SPACE, or used on its own, without pulling in ESCAPE_SPECIAL's
> > > quote and control-character handling.
> >
> > But why? I believe it can be done in the current implementation using
> > the last argument @only (id est use the list of the characters you
> > want to escape).
>
> Ah, that's my fault for suggesting this and not noticing there was way to
> negatively filter them.

No problem, not a big issue :-)

I think here is the list of those 4 characters that we want to escape
should be passed along with ESCAPE_SPACE | ESCAPE_SPECIAL. This will
get exact code behaviour as of today. In current patch 2/2 AFAICS the
additional SPACE-class characters might have also been escaped which
was not in the original implementation (not sure if it's desired
change or not).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-09-28 13:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-26 20:56 [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Muhammad Bilal
2026-09-26 20:56 ` [PATCH 2/2] platform/x86: hp-bioscfg: fix heap OOB read and non-ASCII truncation in hp_get_string_from_buffer() Muhammad Bilal
2026-09-28 11:32   ` Ilpo Järvinen
2026-09-28  8:11 ` [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() Andy Shevchenko
2026-09-28  8:17   ` Andy Shevchenko
2026-09-28 12:59   ` Ilpo Järvinen
2026-09-28 13:38     ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®