linux-hardening - Re: [PATCH 3/4] kdb: Replace deprecated strcpy() with memcpy() in parse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAD=FV=W7Qhz4dNnhb3t4+Bz3Fk5iKxRNwrq8u4X73tpGsg=hyA@mail.gmail.com>
Date: Mon, 18 Aug 2025 13:35:06 -0700
From: Doug Anderson <dianders@...omium.org>
To: Thorsten Blum <thorsten.blum@...ux.dev>
Cc: Jason Wessel <jason.wessel@...driver.com>, Daniel Thompson <danielt@...nel.org>, 
	Nir Lichtman <nir@...htman.org>, Yuran Pereira <yuran.pereira@...mail.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-hardening@...r.kernel.org, 
	Daniel Thompson <daniel@...cstar.com>, kgdb-bugreport@...ts.sourceforge.net, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] kdb: Replace deprecated strcpy() with memcpy() in parse_grep()

Hi,

On Mon, Aug 18, 2025 at 11:14 AM Thorsten Blum <thorsten.blum@...ux.dev> wrote:
>
> strcpy() is deprecated; use memcpy() instead.
>
> We can safely use memcpy() because we already know the length of the
> source string 'cp' and that it is guaranteed to be NUL-terminated within
> the first KDB_GREP_STRLEN bytes.
>
> Link: https://github.com/KSPP/linux/issues/88
> Signed-off-by: Thorsten Blum <thorsten.blum@...ux.dev>
> ---
>  kernel/debug/kdb/kdb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 7a4d2d4689a5..cdf91976eb7c 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -860,7 +860,7 @@ static void parse_grep(const char *str)
>                 kdb_printf("search string too long\n");
>                 return;
>         }
> -       strcpy(kdb_grep_string, cp);
> +       memcpy(kdb_grep_string, cp, len + 1);

I'm OK with this:

Reviewed-by: Douglas Anderson <dianders@...omium.org>

A part of me would rather use strscpy() the way it's supposed to be
used to simplify the code a tiny bit... AKA (untested):

-       len = strlen(cp);
+       len = strscpy(kdb_grep_string, cp);
        if (!len)
                return;
-       if (len >= KDB_GREP_STRLEN) {
+       if (len < 0) {
                kdb_printf("search string too long\n");
                return;
        }
-       strcpy(kdb_grep_string, cp);

I guess this does "change" the behavior in that `kdb_grep_string` will
still be set to the empty string in the case that "len" was 0 and will
still be set to a truncated copy in the case that "len" was too big,
but based on my quick analysis of the code that should be fine.

In any case, we're getting pretty far into nitpicks here, so I'm also
OK w/ just leaving it the way you have it. ;-)