回复: [PATCH] app/testpmd: avoid cmdline use-after-free on SIGINT

[Sunyang Wu]

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 at intel.com> 
发送时间: 2026年4月27日 18:02
收件人: Sunyang Wu <sunyang.wu at jaguarmicro.com>
抄送: dev at dpdk.org; aman.deep.singh at intel.com; stable at 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 at dpdk.org
>
> Signed-off-by: Sunyang Wu <sunyang.wu at 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