DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT
@ 2026-04-27  9:10 Sunyang Wu
  2026-04-27 10:02 ` Bruce Richardson
  0 siblings, 1 reply; 4+ messages in thread
From: Sunyang Wu @ 2026-04-27  9:10 UTC (permalink / raw)
  To: dev; +Cc: aman.deep.singh, stable

When testpmd runs in interactive mode, SIGINT is handled by setting
the quit flag and calling prompt_exit() so the cmdline input path can
be interrupted.

However, prompt() frees the cmdline object with cmdline_stdin_exit()
after cmdline_interact() returns, while the global testpmd_cl pointer
may still be observed by a later signal during shutdown. If SIGINT
arrives after the cmdline object is freed, prompt_exit() may call
cmdline_quit() on stale state and trigger a use-after-free.

Keep the existing prompt_exit() behavior so interactive input can
still be cancelled, but store the cmdline object in a local variable
and clear testpmd_cl before freeing it.

This preserves the interactive-mode fix introduced for Windows while
avoiding a shutdown-time use-after-free.

Fixes: f1d0993e034e ("app/testpmd: fix interactive mode on Windows")
Cc: stable@dpdk.org

Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
---
 app/test-pmd/cmdline.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index c5abeb5730..e3ed0f1865 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -14500,22 +14500,28 @@ cmdline_read_from_file(const char *filename, bool echo)
 void
 prompt_exit(void)
 {
-	cmdline_quit(testpmd_cl);
+	if (testpmd_cl != NULL)
+		cmdline_quit(testpmd_cl);
 }
 
 /* prompt function, called from main on MAIN lcore */
 void
 prompt(void)
 {
-	testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
-	if (testpmd_cl == NULL) {
+	struct cmdline *cl;
+
+	cl = cmdline_stdin_new(main_ctx, "testpmd> ");
+	if (cl == NULL) {
 		fprintf(stderr,
 			"Failed to create stdin based cmdline context\n");
 		return;
 	}
 
-	cmdline_interact(testpmd_cl);
-	cmdline_stdin_exit(testpmd_cl);
+	testpmd_cl = cl;
+	cmdline_interact(cl);
+	/* Clear global pointer before freeing cmdline object. */
+	testpmd_cl = NULL;
+	cmdline_stdin_exit(cl);
 }
 
 void
-- 
2.19.0.rc0.windows.1


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

* Re: [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT
  2026-04-27  9:10 [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT Sunyang Wu
@ 2026-04-27 10:02 ` Bruce Richardson
  2026-04-27 10:33   ` 回复: " Sunyang Wu
  0 siblings, 1 reply; 4+ messages in thread
From: Bruce Richardson @ 2026-04-27 10:02 UTC (permalink / raw)
  To: Sunyang Wu; +Cc: dev, aman.deep.singh, stable

On Mon, Apr 27, 2026 at 05:10:55PM +0800, Sunyang Wu wrote:
> When testpmd runs in interactive mode, SIGINT is handled by setting
> the quit flag and calling prompt_exit() so the cmdline input path can
> be interrupted.
> 
> However, prompt() frees the cmdline object with cmdline_stdin_exit()
> after cmdline_interact() returns, while the global testpmd_cl pointer
> may still be observed by a later signal during shutdown. If SIGINT
> arrives after the cmdline object is freed, prompt_exit() may call
> cmdline_quit() on stale state and trigger a use-after-free.
> 
> Keep the existing prompt_exit() behavior so interactive input can
> still be cancelled, but store the cmdline object in a local variable
> and clear testpmd_cl before freeing it.
> 
> This preserves the interactive-mode fix introduced for Windows while
> avoiding a shutdown-time use-after-free.
> 
> Fixes: f1d0993e034e ("app/testpmd: fix interactive mode on Windows")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
> ---
>  app/test-pmd/cmdline.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index c5abeb5730..e3ed0f1865 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -14500,22 +14500,28 @@ cmdline_read_from_file(const char *filename, bool echo)
>  void
>  prompt_exit(void)
>  {
> -	cmdline_quit(testpmd_cl);
> +	if (testpmd_cl != NULL)
> +		cmdline_quit(testpmd_cl);
>  }
>  
>  /* prompt function, called from main on MAIN lcore */
>  void
>  prompt(void)
>  {
> -	testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
> -	if (testpmd_cl == NULL) {
> +	struct cmdline *cl;
> +
> +	cl = cmdline_stdin_new(main_ctx, "testpmd> ");
> +	if (cl == NULL) {
>  		fprintf(stderr,
>  			"Failed to create stdin based cmdline context\n");
>  		return;
>  	}
>  
> -	cmdline_interact(testpmd_cl);
> -	cmdline_stdin_exit(testpmd_cl);
> +	testpmd_cl = cl;
> +	cmdline_interact(cl);
> +	/* Clear global pointer before freeing cmdline object. */
> +	testpmd_cl = NULL;
> +	cmdline_stdin_exit(cl);
>  }

Do you need some memory barriers in this code to guarantee that the NULL
pointer is visible to other threads before you start calling the exit
function?

/Bruce

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

* 回复: [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT
  2026-04-27 10:02 ` Bruce Richardson
@ 2026-04-27 10:33   ` Sunyang Wu
  2026-04-27 10:43     ` Bruce Richardson
  0 siblings, 1 reply; 4+ messages in thread
From: Sunyang Wu @ 2026-04-27 10:33 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: dev@dpdk.org, aman.deep.singh@intel.com, stable@dpdk.org

Hi Bruce,

Thanks for the review.

I do not think a memory barrier alone would be sufficient here.

In the intended case, prompt_exit() is used from the signal path to
interrupt the thread currently running prompt(). In that case, the
NULL store is already ordered before cmdline_stdin_exit(). However,
for a later signal or a signal delivered to another thread, a plain
barrier would still not make concurrent access to testpmd_cl safe.

I think the better fix is to keep the existing prompt_exit() behavior,
but use a local cmdline pointer for lifetime management and atomic
load/store for testpmd_cl so the signal path cannot observe freed
state.

If this approach looks reasonable to you, I will send a v2.

Thanks,
Sunyang

-----邮件原件-----
发件人: Bruce Richardson <bruce.richardson@intel.com> 
发送时间: 2026年4月27日 18:02
收件人: Sunyang Wu <sunyang.wu@jaguarmicro.com>
抄送: dev@dpdk.org; aman.deep.singh@intel.com; stable@dpdk.org
主题: Re: [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT

On Mon, Apr 27, 2026 at 05:10:55PM +0800, Sunyang Wu wrote:
> When testpmd runs in interactive mode, SIGINT is handled by setting 
> the quit flag and calling prompt_exit() so the cmdline input path can 
> be interrupted.
>
> However, prompt() frees the cmdline object with cmdline_stdin_exit() 
> after cmdline_interact() returns, while the global testpmd_cl pointer 
> may still be observed by a later signal during shutdown. If SIGINT 
> arrives after the cmdline object is freed, prompt_exit() may call
> cmdline_quit() on stale state and trigger a use-after-free.
>
> Keep the existing prompt_exit() behavior so interactive input can 
> still be cancelled, but store the cmdline object in a local variable 
> and clear testpmd_cl before freeing it.
>
> This preserves the interactive-mode fix introduced for Windows while 
> avoiding a shutdown-time use-after-free.
>
> Fixes: f1d0993e034e ("app/testpmd: fix interactive mode on Windows")
> Cc: stable@dpdk.org
>
> Signed-off-by: Sunyang Wu <sunyang.wu@jaguarmicro.com>
> ---
>  app/test-pmd/cmdline.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 
> c5abeb5730..e3ed0f1865 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -14500,22 +14500,28 @@ cmdline_read_from_file(const char *filename, 
> bool echo)  void
>  prompt_exit(void)
>  {
> -     cmdline_quit(testpmd_cl);
> +     if (testpmd_cl != NULL)
> +             cmdline_quit(testpmd_cl);
>  }
>
>  /* prompt function, called from main on MAIN lcore */  void
>  prompt(void)
>  {
> -     testpmd_cl = cmdline_stdin_new(main_ctx, "testpmd> ");
> -     if (testpmd_cl == NULL) {
> +     struct cmdline *cl;
> +
> +     cl = cmdline_stdin_new(main_ctx, "testpmd> ");
> +     if (cl == NULL) {
>               fprintf(stderr,
>                       "Failed to create stdin based cmdline context\n");
>               return;
>       }
>
> -     cmdline_interact(testpmd_cl);
> -     cmdline_stdin_exit(testpmd_cl);
> +     testpmd_cl = cl;
> +     cmdline_interact(cl);
> +     /* Clear global pointer before freeing cmdline object. */
> +     testpmd_cl = NULL;
> +     cmdline_stdin_exit(cl);
>  }

Do you need some memory barriers in this code to guarantee that the NULL pointer is visible to other threads before you start calling the exit function?

/Bruce

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

* Re: 回复: [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT
  2026-04-27 10:33   ` 回复: " Sunyang Wu
@ 2026-04-27 10:43     ` Bruce Richardson
  0 siblings, 0 replies; 4+ messages in thread
From: Bruce Richardson @ 2026-04-27 10:43 UTC (permalink / raw)
  To: Sunyang Wu; +Cc: dev@dpdk.org, aman.deep.singh@intel.com, stable@dpdk.org

On Mon, Apr 27, 2026 at 10:33:42AM +0000, Sunyang Wu wrote:
> Hi Bruce,
> 
> Thanks for the review.
> 
> I do not think a memory barrier alone would be sufficient here.
> 
> In the intended case, prompt_exit() is used from the signal path to
> interrupt the thread currently running prompt(). In that case, the
> NULL store is already ordered before cmdline_stdin_exit(). However,
> for a later signal or a signal delivered to another thread, a plain
> barrier would still not make concurrent access to testpmd_cl safe.
> 
> I think the better fix is to keep the existing prompt_exit() behavior,
> but use a local cmdline pointer for lifetime management and atomic
> load/store for testpmd_cl so the signal path cannot observe freed
> state.
> 
> If this approach looks reasonable to you, I will send a v2.
> 

Sounds reasonable to me, but I'm not an expert on the behaviour of weakly
ordered platforms! I think do a v2 with your proposal anyway, since it's
likely an improvement over the v1.

/Bruce

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

end of thread, other threads:[~2026-04-27 10:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-27  9:10 [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT Sunyang Wu
2026-04-27 10:02 ` Bruce Richardson
2026-04-27 10:33   ` 回复: " Sunyang Wu
2026-04-27 10:43     ` Bruce Richardson

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