Messages in this thread | | | Date | Mon, 08 Apr 2024 17:38:24 +0200 | From | "Arnd Bergmann" <> | Subject | Re: [PATCH 05/11] test_hexdump: avoid string truncation warning |
| |
On Fri, Mar 29, 2024, at 00:54, Justin Stitt wrote: > 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().
[cut useful explanation] > 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.
You are correct. I do understand the nuances between strncpy() and strscpy(), but I failed to read this file properly.
I'm still not entirely sure, but from my current reading, I think we can just use memcpy() to replace the strncpy() here, as both the input string data_b[] and the output real[TEST_HEXDUMP_BUF_SIZE] are sized to cover every possible 'len' value. This also follows what Linus did for the other original strncpy in b1286ed7158e ("test_hexdump: use memcpy instead of strncpy()").
I've reworked the patch based on that assumption now and rewritten the changelog text accordingly.
Arnd
| |