|SUCCESS| [v3] cmdline: prevent out-of-bounds read in completion buffer

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163509

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-05-01

commit: af75078fece3
Author: Daniil Iskhakov <dish at amicon.ru>
Subject: [PATCH v3] cmdline: prevent out-of-bounds read in completion buffer

tmp_buf is populated by the completion callback and is not guaranteed
to be NUL-terminated.

The code already accounts for this when computing tmp_size with
strnlen(tmp_buf, sizeof(tmp_buf)). However, another loop in the same
path still walks tmp_buf until a NUL byte is found, without checking
the buffer limit.

If the callback writes a full-sized non-NUL-terminated string, the loop
may read past the end of tmp_buf.

Fix this by computing a bounded length for each completion choice before
printing it.

> diff --git a/lib/cmdline/cmdline_rdline.c b/lib/cmdline/cmdline_rdline.c
> index 0a5a399b32..e1770f0a1d 100644
> --- a/lib/cmdline/cmdline_rdline.c
> +++ b/lib/cmdline/cmdline_rdline.c
> @@ -444,8 +444,9 @@ rdline_char_in(struct rdline *rdl, char c)
>  				/* choice */
>  				rdline_puts(rdl, "\r\n");
>  				while (ret) {
> +					tmp_size = strnlen(tmp_buf, sizeof(tmp_buf));

Does this code recompute tmp_size on every loop iteration? If the
complete() callback can modify tmp_buf across iterations, is the
value from the previous iteration being used incorrectly?

Looking at the control flow: the loop calls complete() again after
printing each choice. This means tmp_buf may contain a different
completion string each time through the loop. Can the current patch
introduce a bug where tmp_size from one iteration is used to bound
the printing of tmp_buf from the next iteration?

>  					rdl->write_char(rdl, ' ');
> -					for (i=0 ; tmp_buf[i] ; i++)
> +					for (i = 0; i < tmp_size; i++)
>  						rdl->write_char(rdl, tmp_buf[i]);
>  					rdline_puts(rdl, "\r\n");
>  					ret = rdl->complete(rdl, rdl->left_buf,

After the for loop exits, the code calls complete() again to get the
next choice. If complete() returns non-zero and populates tmp_buf
with a new string, the next iteration uses the old tmp_size value
computed before this complete() call. Does this code print the wrong
number of characters from the new completion string?