lkml.org 
[lkml]   [2024]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH 05/11] test_hexdump: avoid string truncation warning
    Hi,

    On Thu, Mar 28, 2024 at 03:04:49PM +0100, Arnd Bergmann wrote:
    > From: Arnd Bergmann <arnd@arndb.de>
    >
    > gcc can warn when a string is too long to fit into the strncpy()
    > destination buffer, as it is here depending on the function
    > arguments:
    >
    > inlined from 'test_hexdump_prepare_test.constprop' at /home/arnd/arm-soc/lib/test_hexdump.c:116:3:
    > include/linux/fortify-string.h:108:33: error: '__builtin_strncpy' output truncated copying between 0 and 32 bytes from a string of length 32 [-Werror=stringop-truncation]
    > 108 | #define __underlying_strncpy __builtin_strncpy
    > | ^
    > include/linux/fortify-string.h:187:16: note: in expansion of macro '__underlying_strncpy'
    > 187 | return __underlying_strncpy(p, q, size);
    > | ^~~~~~~~~~~~~~~~~~~~
    >
    > As far as I can tell, this is harmless here because the truncation is
    > intentional, but using strscpy_pad() will avoid the warning since gcc
    > does not (yet) know about it.
    >

    We need to be careful. strscpy() or strscpy_pad() are not drop-in
    replacements for strncpy().

    if @l is less than the length of @data_a we might have a problem because
    strscpy_pad() will eagerly assign a NUL-byte to dest[l-1].

    It looks like @l could be less than the length of @data_a judging from:
    size_t len = get_random_u32_inclusive(1, d);


    Let me model the potential behavior before and after, understanding that
    data_a is defined as:

    static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";

    Before (using strncpy):
    p = ['j', 'u', 'n', 'k'] // example destination buffer
    assume @l = 3
    then we are trying to copy ".2." from @data_a, resulting in
    p = ['.', '2', '.', 'k']

    After (using strscpy_pad()):
    p = ['j', 'u', 'n', 'k'] // example destination buffer
    assume @l = 3
    then we are trying to copy ".2." from @data_a, resulting in
    p = ['.', '2', '\0', 'k']

    because strscpy got to the end of its allowed size and didn't find a
    NUL-term from its source string, so it eagerly assigns a NUL-byte to the
    end, essentially truncating our string.


    Here's the responsible code from strscpy's implementation:
    if (res)
    dest[res-1] = '\0';

    https://elixir.bootlin.com/linux/latest/source/lib/string.c#L107

    It is possible I haven't fully considered the context of this change but
    I think using strscpy_pad() will cause these tests to fail, if they
    aren't failing I think we're getting lucky.


    Also, maybe this godbolt example can help demonstrate:
    https://godbolt.org/z/nWGKraTvT

    >
    > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    > ---
    > lib/test_hexdump.c | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
    > index b916801f23a8..c9820122af56 100644
    > --- a/lib/test_hexdump.c
    > +++ b/lib/test_hexdump.c
    > @@ -113,7 +113,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
    > *p++ = ' ';
    > } while (p < test + rs * 2 + rs / gs + 1);
    >
    > - strncpy(p, data_a, l);
    > + strscpy_pad(p, data_a, l);
    > p += l;
    > }
    >
    > --
    > 2.39.2
    >

    Thanks
    Justin

    \
     
     \ /
      Last update: 2024-03-29 00:54    [W:4.041 / U:0.284 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site