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 <[email protected]> 发送时间: 2026年4月27日 18:02 收件人: Sunyang Wu <[email protected]> 抄送: [email protected]; [email protected]; [email protected] 主题: 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: [email protected] > > Signed-off-by: Sunyang Wu <[email protected]> > --- > 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
