From ce6616724fb425d6043d0dad6af996cd7c79bcc4 Mon Sep 17 00:00:00 2001 From: Jann Horn Date: Wed, 5 Jul 2023 23:51:27 +0200 Subject: [PATCH 1/2] ubsan: Clarify Kconfig text for CONFIG_UBSAN_TRAP Make it clearer in the one-line description and the verbose description text that CONFIG_UBSAN_TRAP as currently implemented involves a tradeoff of much less helpful oops messages in exchange for a smaller kernel image. (With the additional effect of turning UBSAN warnings into crashes, which may or may not be desired.) Signed-off-by: Jann Horn Link: https://lore.kernel.org/r/20230705215128.486054-1-jannh@google.com Signed-off-by: Kees Cook --- lib/Kconfig.ubsan | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan index efae7e0119565..59e21bfec188c 100644 --- a/lib/Kconfig.ubsan +++ b/lib/Kconfig.ubsan @@ -13,7 +13,7 @@ menuconfig UBSAN if UBSAN config UBSAN_TRAP - bool "On Sanitizer warnings, abort the running kernel code" + bool "Abort on Sanitizer warnings (smaller kernel but less verbose)" depends on !COMPILE_TEST help Building kernels with Sanitizer features enabled tends to grow @@ -26,6 +26,14 @@ config UBSAN_TRAP the system. For some system builders this is an acceptable trade-off. + Also note that selecting Y will cause your kernel to Oops + with an "illegal instruction" error with no further details + when a UBSAN violation occurs. (Except on arm64, which will + report which Sanitizer failed.) This may make it hard to + determine whether an Oops was caused by UBSAN or to figure + out the details of a UBSAN violation. It makes the kernel log + output less useful for bug reports. + config CC_HAS_UBSAN_BOUNDS_STRICT def_bool $(cc-option,-fsanitize=bounds-strict) help From 8453e7924a1a9130f2b4d2c507de2cdc3892a5b5 Mon Sep 17 00:00:00 2001 From: Azeem Shaikh Date: Tue, 23 May 2023 02:14:25 +0000 Subject: [PATCH 2/2] soc: fsl: qe: Replace all non-returning strlcpy with strscpy strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated [1]. In an effort to remove strlcpy() completely [2], replace strlcpy() here with strscpy(). No return values were used, so direct replacement is safe. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [2] https://github.com/KSPP/linux/issues/89 Signed-off-by: Azeem Shaikh Reviewed-by: Kees Cook Link: https://lore.kernel.org/r/20230523021425.2406309-1-azeemshaikh38@gmail.com Signed-off-by: Kees Cook --- drivers/soc/fsl/qe/qe.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/soc/fsl/qe/qe.c b/drivers/soc/fsl/qe/qe.c index b3c226eb5292f..58746e570d14c 100644 --- a/drivers/soc/fsl/qe/qe.c +++ b/drivers/soc/fsl/qe/qe.c @@ -524,7 +524,7 @@ int qe_upload_firmware(const struct qe_firmware *firmware) * saved microcode information and put in the new. */ memset(&qe_firmware_info, 0, sizeof(qe_firmware_info)); - strlcpy(qe_firmware_info.id, firmware->id, sizeof(qe_firmware_info.id)); + strscpy(qe_firmware_info.id, firmware->id, sizeof(qe_firmware_info.id)); qe_firmware_info.extended_modes = be64_to_cpu(firmware->extended_modes); memcpy(qe_firmware_info.vtraps, firmware->vtraps, sizeof(firmware->vtraps)); @@ -599,7 +599,7 @@ struct qe_firmware_info *qe_get_firmware_info(void) /* Copy the data into qe_firmware_info*/ sprop = of_get_property(fw, "id", NULL); if (sprop) - strlcpy(qe_firmware_info.id, sprop, + strscpy(qe_firmware_info.id, sprop, sizeof(qe_firmware_info.id)); of_property_read_u64(fw, "extended-modes",