[PATCH 1/4] tracing/probe: reject empty immediate strings

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/4] tracing/probe: reject empty immediate strings
@ 2026-03-30  6:29 Pengpeng Hou
  2026-04-01 16:03 ` [PATCH v2] " Pengpeng Hou
  0 siblings, 1 reply; 8+ messages in thread
From: Pengpeng Hou @ 2026-03-30  6:29 UTC (permalink / raw)
  To: rostedt
  Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel,
	pengpeng

parse_probe_arg() treats an argument starting with \\" as an
immediate string and passes arg + 2 to __parse_imm_string(). If the
argument contains only the opener, __parse_imm_string() computes
strlen(str) as 0 and then dereferences str[len - 1], reading one byte
before the string.

Reject empty immediate-string bodies before checking the closing quote.

Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 kernel/trace/trace_probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index e0a5dc86c07e..e1c73065dae5 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
 {
 	size_t len = strlen(str);
 
-	if (str[len - 1] != '"') {
+	if (!len || str[len - 1] != '"') {
 		trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
 		return -EINVAL;
 	}
-- 
2.50.1 (Apple Git-155)


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

* [PATCH v2] tracing/probe: reject empty immediate strings
  2026-03-30  6:29 [PATCH 1/4] tracing/probe: reject empty immediate strings Pengpeng Hou
@ 2026-04-01 16:03 ` Pengpeng Hou
  2026-04-01 17:00   ` Steven Rostedt
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Pengpeng Hou @ 2026-04-01 16:03 UTC (permalink / raw)
  To: rostedt
  Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel,
	pengpeng

parse_probe_arg() accepts quoted immediate strings and passes the body
after the opening quote to __parse_imm_string(). That helper currently
computes strlen(str) and immediately dereferences str[len - 1], which
underflows when the body is empty.

Reject empty immediate strings before checking for the closing quote.

Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
Changes since v1:
- resend as a standalone patch instead of part of an accidental
  cross-subsystem 1/4 series

 kernel/trace/trace_probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index e0a5dc86c07e..e1c73065dae5 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
 {
 	size_t len = strlen(str);
 
-	if (str[len - 1] != '"') {
+	if (!len || str[len - 1] != '"') {
 		trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
 		return -EINVAL;
 	}
-- 
2.50.1 (Apple Git-155)


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

* Re: [PATCH v2] tracing/probe: reject empty immediate strings
  2026-04-01 16:03 ` [PATCH v2] " Pengpeng Hou
@ 2026-04-01 17:00   ` Steven Rostedt
  2026-04-04  0:33   ` Masami Hiramatsu
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Steven Rostedt @ 2026-04-01 17:00 UTC (permalink / raw)
  To: Pengpeng Hou
  Cc: mhiramat, mathieu.desnoyers, linux-trace-kernel, linux-kernel

On Thu,  2 Apr 2026 00:03:15 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:

> parse_probe_arg() accepts quoted immediate strings and passes the body
> after the opening quote to __parse_imm_string(). That helper currently
> computes strlen(str) and immediately dereferences str[len - 1], which
> underflows when the body is empty.
> 
> Reject empty immediate strings before checking for the closing quote.
> 
> Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve

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

* Re: [PATCH v2] tracing/probe: reject empty immediate strings
  2026-04-01 16:03 ` [PATCH v2] " Pengpeng Hou
  2026-04-01 17:00   ` Steven Rostedt
@ 2026-04-04  0:33   ` Masami Hiramatsu
  2026-04-06  1:11     ` Masami Hiramatsu
  2026-04-06  1:24   ` Masami Hiramatsu
  2026-04-06  8:20   ` Pengpeng Hou
  3 siblings, 1 reply; 8+ messages in thread
From: Masami Hiramatsu @ 2026-04-04  0:33 UTC (permalink / raw)
  To: Pengpeng Hou
  Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
	linux-kernel

On Thu,  2 Apr 2026 00:03:15 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:

> parse_probe_arg() accepts quoted immediate strings and passes the body
> after the opening quote to __parse_imm_string(). That helper currently
> computes strlen(str) and immediately dereferences str[len - 1], which
> underflows when the body is empty.
> 
> Reject empty immediate strings before checking for the closing quote.
> 

Oops, good catch!

Let me pick it.

Thank you!

> Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1:
> - resend as a standalone patch instead of part of an accidental
>   cross-subsystem 1/4 series
> 
>  kernel/trace/trace_probe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index e0a5dc86c07e..e1c73065dae5 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
>  {
>  	size_t len = strlen(str);
>  
> -	if (str[len - 1] != '"') {
> +	if (!len || str[len - 1] != '"') {
>  		trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
>  		return -EINVAL;
>  	}
> -- 
> 2.50.1 (Apple Git-155)
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH v2] tracing/probe: reject empty immediate strings
  2026-04-04  0:33   ` Masami Hiramatsu
@ 2026-04-06  1:11     ` Masami Hiramatsu
  2026-04-06  1:20       ` Masami Hiramatsu
  0 siblings, 1 reply; 8+ messages in thread
From: Masami Hiramatsu @ 2026-04-06  1:11 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Pengpeng Hou, rostedt, mathieu.desnoyers, linux-trace-kernel,
	linux-kernel

Hi,

On Sat, 4 Apr 2026 09:33:59 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index e0a5dc86c07e..e1c73065dae5 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c
> > @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
> >  {
> >  	size_t len = strlen(str);
> >  
> > -	if (str[len - 1] != '"') {

It seems that this is not correct fix, because __parse_imm_string()
is only called from below code:

	case '\\':	/* Immediate value */
		if (arg[1] == '"') {	/* Immediate string */
			ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
			if (ret)

Thus the call-site already checked the double-quotation.
This means this if block itself is meaningless.

Thanks,

> > +	if (!len || str[len - 1] != '"') {
> >  		trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
> >  		return -EINVAL;
> >  	}
> > -- 
> > 2.50.1 (Apple Git-155)
> > 
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH v2] tracing/probe: reject empty immediate strings
  2026-04-06  1:11     ` Masami Hiramatsu
@ 2026-04-06  1:20       ` Masami Hiramatsu
  0 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2026-04-06  1:20 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Pengpeng Hou, rostedt, mathieu.desnoyers, linux-trace-kernel,
	linux-kernel

On Mon, 6 Apr 2026 10:11:24 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> Hi,
> 
> On Sat, 4 Apr 2026 09:33:59 +0900
> Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > > index e0a5dc86c07e..e1c73065dae5 100644
> > > --- a/kernel/trace/trace_probe.c
> > > +++ b/kernel/trace/trace_probe.c
> > > @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
> > >  {
> > >  	size_t len = strlen(str);
> > >  
> > > -	if (str[len - 1] != '"') {
> 
> It seems that this is not correct fix, because __parse_imm_string()
> is only called from below code:
> 
> 	case '\\':	/* Immediate value */
> 		if (arg[1] == '"') {	/* Immediate string */
> 			ret = __parse_imm_string(arg + 2, &tmp, ctx->offset + 2);
> 			if (ret)
> 
> Thus the call-site already checked the double-quotation.
> This means this if block itself is meaningless.

Nevermind, this fix is correct. But the title is not correct because
we still can specify an empty string (\""), but this rejects non-closed
empty immediate string (\").

Thank you,


> 
> Thanks,
> 
> > > +	if (!len || str[len - 1] != '"') {
> > >  		trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
> > >  		return -EINVAL;
> > >  	}
> > > -- 
> > > 2.50.1 (Apple Git-155)
> > > 
> > 
> > 
> > -- 
> > Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH v2] tracing/probe: reject empty immediate strings
  2026-04-01 16:03 ` [PATCH v2] " Pengpeng Hou
  2026-04-01 17:00   ` Steven Rostedt
  2026-04-04  0:33   ` Masami Hiramatsu
@ 2026-04-06  1:24   ` Masami Hiramatsu
  2026-04-06  8:20   ` Pengpeng Hou
  3 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2026-04-06  1:24 UTC (permalink / raw)
  To: Pengpeng Hou
  Cc: rostedt, mhiramat, mathieu.desnoyers, linux-trace-kernel,
	linux-kernel

I would like to suggest the subject as

"tracing/probe: Reject non-closed empty immediate strings"


On Thu,  2 Apr 2026 00:03:15 +0800
Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:

> parse_probe_arg() accepts quoted immediate strings and passes the body
> after the opening quote to __parse_imm_string(). That helper currently
> computes strlen(str) and immediately dereferences str[len - 1], which
> underflows when the body is empty.
                                   ^ and not closed with double-quotation.

> 
> Reject empty immediate strings before checking for the closing quote.
        ^ non-closed.

Thank you,

> 
> Fixes: a42e3c4de964 ("tracing/probe: Add immediate string parameter support")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> Changes since v1:
> - resend as a standalone patch instead of part of an accidental
>   cross-subsystem 1/4 series
> 
>  kernel/trace/trace_probe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> index e0a5dc86c07e..e1c73065dae5 100644
> --- a/kernel/trace/trace_probe.c
> +++ b/kernel/trace/trace_probe.c
> @@ -1068,7 +1068,7 @@ static int __parse_imm_string(char *str, char **pbuf, int offs)
>  {
>  	size_t len = strlen(str);
>  
> -	if (str[len - 1] != '"') {
> +	if (!len || str[len - 1] != '"') {
>  		trace_probe_log_err(offs + len, IMMSTR_NO_CLOSE);
>  		return -EINVAL;
>  	}
> -- 
> 2.50.1 (Apple Git-155)
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

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

* Re: [PATCH v2] tracing/probe: reject empty immediate strings
  2026-04-01 16:03 ` [PATCH v2] " Pengpeng Hou
                     ` (2 preceding siblings ...)
  2026-04-06  1:24   ` Masami Hiramatsu
@ 2026-04-06  8:20   ` Pengpeng Hou
  3 siblings, 0 replies; 8+ messages in thread
From: Pengpeng Hou @ 2026-04-06  8:20 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Masami Hiramatsu, Mathieu Desnoyers, linux-trace-kernel,
	linux-kernel, pengpeng

Hi Masami,

Thanks, you're right.

The fix is for a non-closed empty immediate string, not for an empty
immediate string in general. Your suggested subject and wording are more
accurate.

Please feel free to adjust the subject/changelog that way when applying.
If you would prefer a respin from my side, I can resend it.

Thanks,
Pengpeng

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

end of thread, other threads:[~2026-04-06  2:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-30  6:29 [PATCH 1/4] tracing/probe: reject empty immediate strings Pengpeng Hou
2026-04-01 16:03 ` [PATCH v2] " Pengpeng Hou
2026-04-01 17:00   ` Steven Rostedt
2026-04-04  0:33   ` Masami Hiramatsu
2026-04-06  1:11     ` Masami Hiramatsu
2026-04-06  1:20       ` Masami Hiramatsu
2026-04-06  1:24   ` Masami Hiramatsu
2026-04-06  8:20   ` Pengpeng Hou

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