linux-kernel - Re: [PATCH v3 1/3] string: Make stpcpy() possible to use

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]

Date:   Wed, 26 Jan 2022 09:38:02 -0700
From:   Nathan Chancellor <nathan@...nel.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Kees Cook <keescook@...omium.org>,
        Francis Laniel <laniel_francis@...vacyrequired.com>,
        Petr Mladek <pmladek@...e.com>, linux-kernel@...r.kernel.org,
        Andy Shevchenko <andy@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        Rasmus Villemoes <linux@...musvillemoes.dk>,
        kernel test robot <lkp@...el.com>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        Nathan Chancellor <natechancellor@...il.com>
Subject: Re: [PATCH v3 1/3] string: Make stpcpy() possible to use

On Wed, Jan 26, 2022 at 04:19:15PM +0200, Andy Shevchenko wrote:
> It is a good rule to avoid submitting code without users.

While I agree with the sentiment in the general case, I don't think that
it applies in this case and this comment should be dropped. The message
of the commit this fixes and the comment right above the declaration
both make it pretty obvious why this interface was added with no in-tree
users and why the declaration was placed right above the definition.

> Currently the stpcpy() is unusable due to missed declaration.
> Any attempts to use it will bring something like:
> 
>   error: implicit declaration of function ‘stpcpy’ [-Werror=implicit-function-declaration]
> 
> Move declaration to the header and guard it as other string functions.
> 
> Fixes: 1e1b6d63d634 ("lib/string.c: implement stpcpy")
> Reported-by: kernel test robot <lkp@...el.com>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Cc: Nathan Chancellor <natechancellor@...il.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>

Regardless, the commit itself seems fine from a technical standpoint. I
won't comment on whether or not this interface should be opened up.

Reviewed-by: Nathan Chancellor <nathan@...nel.org>

> ---
> v3: new patch to fix reported issue
>  include/linux/string.h | 3 +++
>  lib/string.c           | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index b6572aeca2f5..b1aeb3475396 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -31,6 +31,9 @@ size_t strlcpy(char *, const char *, size_t);
>  #ifndef __HAVE_ARCH_STRSCPY
>  ssize_t strscpy(char *, const char *, size_t);
>  #endif
> +#ifndef __HAVE_ARCH_STPCPY
> +char *stpcpy(char *__restrict__ dest, const char *__restrict__ src);
> +#endif
>  
>  /* Wraps calls to strscpy()/memset(), no arch specific code required */
>  ssize_t strscpy_pad(char *dest, const char *src, size_t count);
> diff --git a/lib/string.c b/lib/string.c
> index 485777c9da83..4ecb8ec1fdd1 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -233,6 +233,7 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
>  EXPORT_SYMBOL(strscpy);
>  #endif
>  
> +#ifndef __HAVE_ARCH_STPCPY
>  /**
>   * stpcpy - copy a string from src to dest returning a pointer to the new end
>   *          of dest, including src's %NUL-terminator. May overrun dest.
> @@ -248,7 +249,6 @@ EXPORT_SYMBOL(strscpy);
>   * not recommended for usage. Instead, its definition is provided in case
>   * the compiler lowers other libcalls to stpcpy.
>   */
> -char *stpcpy(char *__restrict__ dest, const char *__restrict__ src);
>  char *stpcpy(char *__restrict__ dest, const char *__restrict__ src)
>  {
>  	while ((*dest++ = *src++) != '\0')
> @@ -256,6 +256,7 @@ char *stpcpy(char *__restrict__ dest, const char *__restrict__ src)
>  	return --dest;
>  }
>  EXPORT_SYMBOL(stpcpy);
> +#endif
>  
>  #ifndef __HAVE_ARCH_STRCAT
>  /**
> -- 
> 2.34.1
> 
>