* [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