* [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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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 2026-09-28 20:08 ` Muhammad Bilal 0 siblings, 1 reply; 8+ 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] 8+ messages in thread
* Re: [PATCH 1/2] lib/string_helpers: add ESCAPE_BACKSLASH flag to string_escape_mem() 2026-09-28 13:38 ` Andy Shevchenko @ 2026-09-28 20:08 ` Muhammad Bilal 0 siblings, 0 replies; 8+ messages in thread From: Muhammad Bilal @ 2026-09-28 20:08 UTC (permalink / raw) To: Andy Shevchenko Cc: Ilpo Järvinen, Kees Cook, Andy Shevchenko, linux-hardening, LKML Thanks for review. I've send v2.[1] Link : https://lore.kernel.org/all/20260928200318.63383-1-meatuni001@gmail.com/ [1] On Mon, Sep 28, 2026 at 6:39 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > 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] 8+ messages in thread
end of thread, other threads:[~2026-09-28 20:08 UTC | newest] Thread overview: 8+ 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 2026-09-28 20:08 ` Muhammad Bilal
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®