linux-kernel - Re: [PATCH] Use strscpy() instead of the deprecated strlcpy()

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: <3CA737FF-2F21-42CA-BF95-5F0341F6B68B@linux.vnet.ibm.com>
Date: Tue, 5 Nov 2024 17:54:38 +0530
From: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
To: Daniel Yang <danielyangkang@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        "Liang, Kan" <kan.liang@...ux.intel.com>,
        "open list:PERFORMANCE EVENTS SUBSYSTEM" <linux-perf-users@...r.kernel.org>,
        "open list:PERFORMANCE EVENTS SUBSYSTEM" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Use strscpy() instead of the deprecated strlcpy()



> On 5 Nov 2024, at 12:30 PM, Daniel Yang <danielyangkang@...il.com> wrote:
> 
> The latest kernel doc:
> https://www.kernel.org/doc/html/latest/process/deprecated.html
> recommends replacing strlcpy with strscpy. Since the return value of
> strlcpy is not used, replacing with strscpy should be safe.
> 
> Signed-off-by: Daniel Yang <danielyangkang@...il.com>
> ---
> tools/perf/util/symbol.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index 3bbf173ad..660ed5626 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1893,7 +1893,7 @@ int dso__load(struct dso *dso, struct map *map)
> char *new_name = dso__filename_with_chroot(dso, name);
> if (new_name) {
> is_reg = is_regular_file(new_name);
> - strlcpy(name, new_name, PATH_MAX);
> + strscpy(name, new_name, PATH_MAX);
> free(new_name);

Hi,

This hits a compilation fail:

In file included from util/symbol.c:10:
util/symbol.c: In function ‘dso__load’:
/home/athira/perf-tools-next/tools/include/linux/string.h:15:17: error: too many arguments to function ‘strcpy’
   15 | #define strscpy strcpy
      |                 ^~~~~~
util/symbol.c:1896:33: note: in expansion of macro ‘strscpy’
 1896 |                                 strscpy(name, new_name, PATH_MAX);
      |                                 ^~~~~~~
In file included from /usr/include/features.h:490,
                 from /usr/include/dirent.h:25,
                 from util/symbol.c:2:
/usr/include/bits/string_fortified.h:77:1: note: declared here
   77 | __NTH (strcpy (char *__restrict __dest, const char *__restrict __src))
      | ^~~~~



“strscpy" in perf tools is defined here: tools/include/linux/string.h as “strcpy”

#define strscpy strcpy

And this is introduced from below commit:

commit 9e3d665384fca2a1c56283c7a79a968243ef4614
Author: Wei Yang <richard.weiyang@...il.com>
Date:   Tue Aug 6 01:03:19 2024 +0000

    memblock test: fix implicit declaration of function 'strscpy'
    
    Commit 1e4c64b71c9b ("mm/memblock: Add "reserve_mem" to reserved named
    memory at boot up") introduce the usage of strscpy, which breaks the
    memblock test.
    
    Let's define it as strcpy in userspace to fix it.
    
    Signed-off-by: Wei Yang <richard.weiyang@...il.com>
    Link: https://lore.kernel.org/r/20240806010319.29194-5-richard.weiyang@gmail.com
    Signed-off-by: Mike Rapoport (Microsoft) <rppt@...nel.org>

diff --git a/tools/include/linux/string.h b/tools/include/linux/string.h
index fb8eda3019b5..41e7fa734922 100644
--- a/tools/include/linux/string.h
+++ b/tools/include/linux/string.h
@@ -12,6 +12,8 @@ void argv_free(char **argv);
 
 int strtobool(const char *s, bool *res);
 
+#define strscpy strcpy
+
 /*
  * glibc based builds needs the extern while uClibc doesn't.
  * However uClibc headers also define __GLIBC__ hence the hack below


So the strscpy here is defined as strcpy itself. 

Ian discussed about having an strscpy shim for perf here :
https://lore.kernel.org/linux-perf-users/172892779710.897882.14949543082561189584.b4-ty@kernel.org/T/#meaa2f8d993c6db435f9ed399dc1bf10132a31292

So IIUC, we are yet to have the safe strscpy in the tools perf.
Please correct if I am wrong. Were you able to compile the patch in your setup or am I missing something ?

Thanks
Athira

> }
> }
> -- 
> 2.39.2
> 
>