Hi,
On Mon, Aug 11, 2025 at 10:04 AM Thorsten Blum <thorsten.b...@linux.dev> wrote:
>
> strcpy() is deprecated; use strscpy() instead.
>
> Use the return value of strscpy() instead of calling strlen() again.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.b...@linux.dev>
> ---
> kernel/debug/kdb/kdb_io.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
> index 9b11b10b120c..2062494c413b 100644
> --- a/kernel/debug/kdb/kdb_io.c
> +++ b/kernel/debug/kdb/kdb_io.c
> @@ -732,8 +732,7 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt,
> va_list ap)
> * Shift the buffer left.
> */
> *cphold = replaced_byte;
> - strcpy(kdb_buffer, cphold);
> - len = strlen(kdb_buffer);
> + len = strscpy(kdb_buffer, cphold);
> next_avail = kdb_buffer + len;
> size_avail = sizeof(kdb_buffer) - len;
> goto kdb_print_out;
It made me a little nervous that you're not checking for the fact that
strscpy() could return an error code. Without the check you're just
replacing one type of problem (buffer overflow) with another type (the
code running with a negative length). IMO in cases like this either
leave the strlen() in there or check the return value for errors.
...so I looked a little deeper here to see if the buffer overflow was
actually possible to begin with. Looking, I _think_ this is true:
* `cp` is a pointer into `kdb_buffer` (location of first '\n')
* `cphold` and `cp` are equal at this point.
...so you're guaranteed not to overflow because the destination and
source overlap. ...but that means we shouldn't have been using
strcpy() either way. Both strcpy() and strscpy() say that their
behaviors are undefined if the src/dest overlap. This means that
really the right fix is to use memmove().
The above is based solely on code inspection w/ no testing. If I got
it wrong, let me know.
> @@ -872,8 +871,7 @@ int vkdb_printf(enum kdb_msgsrc src, const char *fmt,
> va_list ap)
> */
> if (kdb_grepping_flag && !suspend_grep) {
> *cphold = replaced_byte;
> - strcpy(kdb_buffer, cphold);
> - len = strlen(kdb_buffer);
> + len = strscpy(kdb_buffer, cphold);
> next_avail = kdb_buffer + len;
> size_avail = sizeof(kdb_buffer) - len;
I believe the above case is similar.
-Doug
