linux-kernel - Re: [PATCH 3/4] tools/nolibc: Fix strlcpy() return code and size usage

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, 14 Feb 2024 12:50:53 -0300
From: Rodrigo Campos <rodrigo@...g.com.ar>
To: Willy Tarreau <w@....eu>
Cc: Thomas Weißschuh <linux@...ssschuh.net>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] tools/nolibc: Fix strlcpy() return code and size
 usage

On 2/11/24 12:08, Willy Tarreau wrote:
> Hi Rodrigo,
> 
> It's good, but for the same reason as the previous one, I'm getting
> smaller code by doing less in the loop. Also calling strlen() here
> looks expensive, I'm seeing that the compiler inlined it nevertheless
> and did it in a dep-optimized way due to the asm statement. That
> results in 67 bytes total while a simpler version gives 47.
> 
> If I explicitly mark strlen() __attribute__((noinline)) that prevents
> it from doing so starting with gcc-10, where it correctly places a jump
> from strlcpy() to strlen() and ends up with 50 bytes (vs 44 for the alt
> one). The other one I can propose is directly derived from the other
> strlcat() variant, which first performs the copy and starts to count:
> 
> size_t strlcpy(char *dst, const char *src, size_t size)
> {
>          size_t len;
> 
>          for (len = 0; len < size; len++) {
>                  if (!(dst[len] = src[len]))
>                          return len;
>          }
> 
>          /* end of src not found before size */
>          if (size)
>                  dst[size - 1] = '\0';
> 
>          while (src[len])
>                  len++;
> 
>          return len;
> }
> 
> Just let me know what you think. 

This is one is very nice, thanks!

Sorry I didn't think about the size at all when writing the functions :)

We can change the loop to be:

         for (len = 0; len < size; len++) {
                 dst[len] = src[len];
                 if (!dst[len])
                         break;
         }

That IMHO it is slightly more readable and makes it only 2 bytes longer 
here.

What do you think? I'm fine with both, of course.


If I resend, shall I add a suggested-by or directly you as the author?



Best,
Rodrigo