linux-kernel - Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings

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: <CAG4es9WQOOga8Oh9BOjr_JXy5gcUzVN0iTtfjN_HVdRj7_G7iQ@mail.gmail.com>
Date: Tue, 2 Jul 2024 11:12:12 -0700
From: Edward Liaw <edliaw@...gle.com>
To: John Hubbard <jhubbard@...dia.com>
Cc: Shuah Khan <shuah@...nel.org>, "Jason A . Donenfeld" <Jason@...c4.com>, Andy Lutomirski <luto@...nel.org>, 
	Mark Brown <broonie@...nel.org>, Vincenzo Frascino <vincenzo.frascino@....com>, 
	Colin Ian King <colin.i.king@...il.com>, Valentin Obst <kernel@...entinobst.de>, 
	linux-kselftest@...r.kernel.org, LKML <linux-kernel@...r.kernel.org>, 
	llvm@...ts.linux.dev, Carlos Llamas <cmllamas@...gle.com>, 
	Muhammad Usama Anjum <usama.anjum@...labora.com>
Subject: Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and warnings

On Fri, Jun 14, 2024 at 4:31 PM John Hubbard <jhubbard@...dia.com> wrote:
>
> When building with clang, via:
>
>     make LLVM=1 -C tools/testing/selftests
>
> ...there are several warnings, and an error. This fixes all of those and
> allows these tests to run and pass.
>
> 1. Fix linker error (undefined reference to memcpy) by providing a local
>    version of memcpy.
>
> 2. clang complains about using this form:
>
>     if (g = h & 0xf0000000)
>
> ...so factor out the assignment into a separate step.
>
> 3. The code is passing a signed const char* to elf_hash(), which expects
>    a const unsigned char *. There are several callers, so fix this at
>    the source by allowing the function to accept a signed argument, and
>    then converting to unsigned operations, once inside the function.
>
> 4. clang doesn't have __attribute__((externally_visible)) and generates
>    a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>    to require that attribute in order to build, run and pass tests here,
>    so remove it.
>
> Reviewed-by: Carlos Llamas <cmllamas@...gle.com>
> Reviewed-by: Edward Liaw <edliaw@...gle.com>
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@...labora.com>
> Signed-off-by: John Hubbard <jhubbard@...dia.com>
> ---
>  tools/testing/selftests/vDSO/parse_vdso.c      | 16 +++++++++++-----
>  .../selftests/vDSO/vdso_standalone_test_x86.c  | 18 ++++++++++++++++--
>  2 files changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
> index 413f75620a35..4ae417372e9e 100644
> --- a/tools/testing/selftests/vDSO/parse_vdso.c
> +++ b/tools/testing/selftests/vDSO/parse_vdso.c
> @@ -55,14 +55,20 @@ static struct vdso_info
>         ELF(Verdef) *verdef;
>  } vdso_info;
>
> -/* Straight from the ELF specification. */
> -static unsigned long elf_hash(const unsigned char *name)
> +/*
> + * Straight from the ELF specification...and then tweaked slightly, in order to
> + * avoid a few clang warnings.
> + */
> +static unsigned long elf_hash(const char *name)
>  {
>         unsigned long h = 0, g;
> -       while (*name)
> +       const unsigned char *uch_name = (const unsigned char *)name;
> +
> +       while (*uch_name)
>         {
> -               h = (h << 4) + *name++;
> -               if (g = h & 0xf0000000)
> +               h = (h << 4) + *uch_name++;
> +               g = h & 0xf0000000;
> +               if (g)
>                         h ^= g >> 24;
>                 h &= ~g;
>         }
> diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> index 8a44ff973ee1..27f6fdf11969 100644
> --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> @@ -18,7 +18,7 @@
>
>  #include "parse_vdso.h"
>
> -/* We need a libc functions... */
> +/* We need some libc functions... */
>  int strcmp(const char *a, const char *b)
>  {
>         /* This implementation is buggy: it never returns -1. */
> @@ -34,6 +34,20 @@ int strcmp(const char *a, const char *b)
>         return 0;
>  }
>
> +/*
> + * The clang build needs this, although gcc does not.
> + * Stolen from lib/string.c.
> + */
> +void *memcpy(void *dest, const void *src, size_t count)
> +{
> +       char *tmp = dest;
> +       const char *s = src;
> +
> +       while (count--)
> +               *tmp++ = *s++;
> +       return dest;
> +}
> +
>  /* ...and two syscalls.  This is x86-specific. */
>  static inline long x86_syscall3(long nr, long a0, long a1, long a2)
>  {
> @@ -70,7 +84,7 @@ void to_base10(char *lastdig, time_t n)
>         }
>  }
>
> -__attribute__((externally_visible)) void c_main(void **stack)
> +void c_main(void **stack)
>  {
>         /* Parse the stack */
>         long argc = (long)*stack;
> --
> 2.45.2
>

Hi John,
Could you try re-submitting this series with the RESEND prefix?

Thanks,
Edward