Skip to content

Commit 0e10872

Browse files
committed
string: Adjust strtomem() logic to allow for smaller sources
Arnd noticed we have a case where a shorter source string is being copied into a destination byte array, but this results in a strnlen() call that exceeds the size of the source. This is seen with -Wstringop-overread: In file included from ../include/linux/uuid.h:11, from ../include/linux/mod_devicetable.h:14, from ../include/linux/cpufeature.h:12, from ../arch/x86/coco/tdx/tdx.c:7: ../arch/x86/coco/tdx/tdx.c: In function 'tdx_panic.constprop': ../include/linux/string.h:284:9: error: 'strnlen' specified bound 64 exceeds source size 60 [-Werror=stringop-overread] 284 | memcpy_and_pad(dest, _dest_len, src, strnlen(src, _dest_len), pad); \ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../arch/x86/coco/tdx/tdx.c:124:9: note: in expansion of macro 'strtomem_pad' 124 | strtomem_pad(message.str, msg, '\0'); | ^~~~~~~~~~~~ Use the smaller of the two buffer sizes when calling strnlen(). When src length is unknown (SIZE_MAX), it is adjusted to use dest length, which is what the original code did. Reported-by: Arnd Bergmann <arnd@arndb.de> Fixes: dfbafa7 ("string: Introduce strtomem() and strtomem_pad()") Tested-by: Arnd Bergmann <arnd@arndb.de> Cc: Andy Shevchenko <andy@kernel.org> Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org>
1 parent faed498 commit 0e10872

File tree

1 file changed

+5
-2
lines changed

1 file changed

+5
-2
lines changed

include/linux/string.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,10 +277,12 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
277277
*/
278278
#define strtomem_pad(dest, src, pad) do { \
279279
const size_t _dest_len = __builtin_object_size(dest, 1); \
280+
const size_t _src_len = __builtin_object_size(src, 1); \
280281
\
281282
BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \
282283
_dest_len == (size_t)-1); \
283-
memcpy_and_pad(dest, _dest_len, src, strnlen(src, _dest_len), pad); \
284+
memcpy_and_pad(dest, _dest_len, src, \
285+
strnlen(src, min(_src_len, _dest_len)), pad); \
284286
} while (0)
285287

286288
/**
@@ -298,10 +300,11 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
298300
*/
299301
#define strtomem(dest, src) do { \
300302
const size_t _dest_len = __builtin_object_size(dest, 1); \
303+
const size_t _src_len = __builtin_object_size(src, 1); \
301304
\
302305
BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \
303306
_dest_len == (size_t)-1); \
304-
memcpy(dest, src, min(_dest_len, strnlen(src, _dest_len))); \
307+
memcpy(dest, src, strnlen(src, min(_src_len, _dest_len))); \
305308
} while (0)
306309

307310
/**

0 commit comments

Comments
 (0)