From 439a1bcac648fe9b59210cde8991fb2acf37bdab Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 19 Sep 2022 15:53:13 -0700 Subject: fortify: Use __builtin_dynamic_object_size() when available Since the commits starting with c37495d6254c ("slab: add __alloc_size attributes for better bounds checking"), the compilers have runtime allocation size hints available in some places. This was immediately available to CONFIG_UBSAN_BOUNDS, but CONFIG_FORTIFY_SOURCE needed updating to explicitly make use of the hints via the associated __builtin_dynamic_object_size() helper. Detect and use the builtin when it is available, increasing the accuracy of the mitigation. When runtime sizes are not available, __builtin_dynamic_object_size() falls back to __builtin_object_size(), leaving the existing bounds checking unchanged. Additionally update the VMALLOC_LINEAR_OVERFLOW LKDTM test to make the hint invisible, otherwise the architectural defense is not exercised (the buffer overflow is detected in the memset() rather than when it crosses the edge of the allocation). Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Nick Desaulniers Cc: Nathan Chancellor Cc: Tom Rix Cc: linux-hardening@vger.kernel.org Cc: llvm@lists.linux.dev Reviewed-by: Miguel Ojeda # include/linux/compiler_attributes.h Signed-off-by: Kees Cook --- drivers/misc/lkdtm/heap.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers') diff --git a/drivers/misc/lkdtm/heap.c b/drivers/misc/lkdtm/heap.c index 62516078a619..0ce4cbf6abda 100644 --- a/drivers/misc/lkdtm/heap.c +++ b/drivers/misc/lkdtm/heap.c @@ -31,6 +31,7 @@ static void lkdtm_VMALLOC_LINEAR_OVERFLOW(void) char *one, *two; one = vzalloc(PAGE_SIZE); + OPTIMIZER_HIDE_VAR(one); two = vzalloc(PAGE_SIZE); pr_info("Attempting vmalloc linear overflow ...\n"); -- cgit v1.2.3 From b5fc3ca3954fd0382bb576a5a295c03a089ac7e3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 5 Jan 2023 13:57:10 -0800 Subject: ARM: ixp4xx: Replace 0-length arrays with flexible arrays Zero-length arrays are deprecated[1]. Replace npe_load_firmware's union of 0-length arrays with flexible arrays. Detected with GCC 13, using -fstrict-flex-arrays=3: drivers/soc/ixp4xx/ixp4xx-npe.c: In function 'npe_load_firmware': drivers/soc/ixp4xx/ixp4xx-npe.c:570:60: warning: array subscript i is outside array bounds of 'u32[0]' {aka 'unsigned int[]'} [-Warray-bounds=] 570 | image->data[i] = swab32(image->data[i]); include/uapi/linux/swab.h:115:54: note: in definition of macro '__swab32' 115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) | ^ drivers/soc/ixp4xx/ixp4xx-npe.c:570:42: note: in expansion of macro 'swab32' 570 | image->data[i] = swab32(image->data[i]); | ^~~~~~ drivers/soc/ixp4xx/ixp4xx-npe.c:522:29: note: while referencing 'data' 522 | u32 data[0]; | ^~~~ [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays Cc: Krzysztof Halasa Cc: Linus Walleij Cc: Arnd Bergmann Cc: "Gustavo A. R. Silva" Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Kees Cook Reviewed-by: Linus Walleij Link: https://lore.kernel.org/r/20230105215706.never.027-kees@kernel.org --- drivers/soc/ixp4xx/ixp4xx-npe.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/soc/ixp4xx/ixp4xx-npe.c b/drivers/soc/ixp4xx/ixp4xx-npe.c index 58240e320c13..5be9988f30ce 100644 --- a/drivers/soc/ixp4xx/ixp4xx-npe.c +++ b/drivers/soc/ixp4xx/ixp4xx-npe.c @@ -519,15 +519,15 @@ int npe_load_firmware(struct npe *npe, const char *name, struct device *dev) u32 id; u32 size; union { - u32 data[0]; - struct dl_block blocks[0]; + DECLARE_FLEX_ARRAY(u32, data); + DECLARE_FLEX_ARRAY(struct dl_block, blocks); }; } *image; struct dl_codeblock { u32 npe_addr; u32 size; - u32 data[0]; + u32 data[]; } *cb; int i, j, err, data_size, instr_size, blocks, table_end; -- cgit v1.2.3 From 4076ea2419cf15bc1e1580f8b24ddf675fbdb02c Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 27 Nov 2022 10:30:41 -0800 Subject: drm/nouveau/disp: Fix nvif_outp_acquire_dp() argument size Both Coverity and GCC with -Wstringop-overflow noticed that nvif_outp_acquire_dp() accidentally defined its second argument with 1 additional element: drivers/gpu/drm/nouveau/dispnv50/disp.c: In function 'nv50_pior_atomic_enable': drivers/gpu/drm/nouveau/dispnv50/disp.c:1813:17: error: 'nvif_outp_acquire_dp' accessing 16 bytes in a region of size 15 [-Werror=stringop-overflow=] 1813 | nvif_outp_acquire_dp(&nv_encoder->outp, nv_encoder->dp.dpcd, 0, 0, false, false); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu/drm/nouveau/dispnv50/disp.c:1813:17: note: referencing argument 2 of type 'u8[16]' {aka 'unsigned char[16]'} drivers/gpu/drm/nouveau/include/nvif/outp.h:24:5: note: in a call to function 'nvif_outp_acquire_dp' 24 | int nvif_outp_acquire_dp(struct nvif_outp *, u8 dpcd[16], | ^~~~~~~~~~~~~~~~~~~~ Avoid these warnings by defining the argument size using the matching define (DP_RECEIVER_CAP_SIZE, 15) instead of having it be a literal (and incorrect) value (16). Reported-by: coverity-bot Addresses-Coverity-ID: 1527269 ("Memory - corruptions") Addresses-Coverity-ID: 1527268 ("Memory - corruptions") Link: https://lore.kernel.org/lkml/202211100848.FFBA2432@keescook/ Link: https://lore.kernel.org/lkml/202211100848.F4C2819BB@keescook/ Fixes: 813443721331 ("drm/nouveau/disp: move DP link config into acquire") Reviewed-by: Lyude Paul Cc: Ben Skeggs Cc: Karol Herbst Cc: David Airlie Cc: Daniel Vetter Cc: Dave Airlie Cc: "Gustavo A. R. Silva" Cc: dri-devel@lists.freedesktop.org Cc: nouveau@lists.freedesktop.org Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221127183036.never.139-kees@kernel.org --- drivers/gpu/drm/nouveau/include/nvif/outp.h | 3 ++- drivers/gpu/drm/nouveau/nvif/outp.c | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/nouveau/include/nvif/outp.h b/drivers/gpu/drm/nouveau/include/nvif/outp.h index 45daadec3c0c..fa76a7b5e4b3 100644 --- a/drivers/gpu/drm/nouveau/include/nvif/outp.h +++ b/drivers/gpu/drm/nouveau/include/nvif/outp.h @@ -3,6 +3,7 @@ #define __NVIF_OUTP_H__ #include #include +#include struct nvif_disp; struct nvif_outp { @@ -21,7 +22,7 @@ int nvif_outp_acquire_rgb_crt(struct nvif_outp *); int nvif_outp_acquire_tmds(struct nvif_outp *, int head, bool hdmi, u8 max_ac_packet, u8 rekey, u8 scdc, bool hda); int nvif_outp_acquire_lvds(struct nvif_outp *, bool dual, bool bpc8); -int nvif_outp_acquire_dp(struct nvif_outp *, u8 dpcd[16], +int nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE], int link_nr, int link_bw, bool hda, bool mst); void nvif_outp_release(struct nvif_outp *); int nvif_outp_infoframe(struct nvif_outp *, u8 type, struct nvif_outp_infoframe_v0 *, u32 size); diff --git a/drivers/gpu/drm/nouveau/nvif/outp.c b/drivers/gpu/drm/nouveau/nvif/outp.c index 7da39f1eae9f..c24bc5eae3ec 100644 --- a/drivers/gpu/drm/nouveau/nvif/outp.c +++ b/drivers/gpu/drm/nouveau/nvif/outp.c @@ -127,7 +127,7 @@ nvif_outp_acquire(struct nvif_outp *outp, u8 proto, struct nvif_outp_acquire_v0 } int -nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[16], +nvif_outp_acquire_dp(struct nvif_outp *outp, u8 dpcd[DP_RECEIVER_CAP_SIZE], int link_nr, int link_bw, bool hda, bool mst) { struct nvif_outp_acquire_v0 args; -- cgit v1.2.3 From 16a738f2f6b31f9edf0691e0f357539f7ac66662 Mon Sep 17 00:00:00 2001 From: Paulo Miguel Almeida Date: Tue, 20 Dec 2022 16:41:15 +1300 Subject: i915/gvt: Replace one-element array with flexible-array member One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in struct gvt_firmware_header and refactor the rest of the code accordingly. Additionally, previous implementation was allocating 8 bytes more than required to represent firmware_header + cfg_space data + mmio data. This helps with the ongoing efforts to tighten the FORTIFY_SOURCE routines on memcpy() and help us make progress towards globally enabling -fstrict-flex-arrays=3 [1]. To make reviewing this patch easier, I'm pasting before/after struct sizes. pahole -C gvt_firmware_header before/drivers/gpu/drm/i915/gvt/firmware.o struct gvt_firmware_header { u64 magic; /* 0 8 */ u32 crc32; /* 8 4 */ u32 version; /* 12 4 */ u64 cfg_space_size; /* 16 8 */ u64 cfg_space_offset; /* 24 8 */ u64 mmio_size; /* 32 8 */ u64 mmio_offset; /* 40 8 */ unsigned char data[1]; /* 48 1 */ /* size: 56, cachelines: 1, members: 8 */ /* padding: 7 */ /* last cacheline: 56 bytes */ }; pahole -C gvt_firmware_header after/drivers/gpu/drm/i915/gvt/firmware.o struct gvt_firmware_header { u64 magic; /* 0 8 */ u32 crc32; /* 8 4 */ u32 version; /* 12 4 */ u64 cfg_space_size; /* 16 8 */ u64 cfg_space_offset; /* 24 8 */ u64 mmio_size; /* 32 8 */ u64 mmio_offset; /* 40 8 */ unsigned char data[]; /* 48 0 */ /* size: 48, cachelines: 1, members: 8 */ /* last cacheline: 48 bytes */ }; As you can see the additional byte of the fake-flexible array (data[1]) forced the compiler to pad the struct but those bytes aren't actually used as first & last bytes (of both cfg_space and mmio) are controlled by the <>_size and <>_offset members present in the gvt_firmware_header struct. Link: https://github.com/KSPP/linux/issues/79 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101836 [1] Signed-off-by: Paulo Miguel Almeida Reviewed-by: Zhenyu Wang Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/Y6Eu2604cqtryP4g@mail.google.com --- drivers/gpu/drm/i915/gvt/firmware.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c index a683c22d5b64..dce93738e98a 100644 --- a/drivers/gpu/drm/i915/gvt/firmware.c +++ b/drivers/gpu/drm/i915/gvt/firmware.c @@ -45,7 +45,7 @@ struct gvt_firmware_header { u64 cfg_space_offset; /* offset in the file */ u64 mmio_size; u64 mmio_offset; /* offset in the file */ - unsigned char data[1]; + unsigned char data[]; }; #define dev_to_drm_minor(d) dev_get_drvdata((d)) @@ -77,7 +77,7 @@ static int expose_firmware_sysfs(struct intel_gvt *gvt) unsigned long size, crc32_start; int ret; - size = sizeof(*h) + info->mmio_size + info->cfg_space_size; + size = offsetof(struct gvt_firmware_header, data) + info->mmio_size + info->cfg_space_size; firmware = vzalloc(size); if (!firmware) return -ENOMEM; -- cgit v1.2.3 From 8500689095a39a8c245ba62709f5e8735668e535 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 5 Jan 2023 15:46:01 -0800 Subject: net/i40e: Replace 0-length array with flexible array Zero-length arrays are deprecated[1]. Replace struct i40e_lump_tracking's "list" 0-length array with a flexible array. Detected with GCC 13, using -fstrict-flex-arrays=3: In function 'i40e_put_lump', inlined from 'i40e_clear_interrupt_scheme' at drivers/net/ethernet/intel/i40e/i40e_main.c:5145:2: drivers/net/ethernet/intel/i40e/i40e_main.c:278:27: warning: array subscript is outside array bounds of 'u16[0]' {aka 'short unsigned int[]'} [-Warray-bounds=] 278 | pile->list[i] = 0; | ~~~~~~~~~~^~~ drivers/net/ethernet/intel/i40e/i40e.h: In function 'i40e_clear_interrupt_scheme': drivers/net/ethernet/intel/i40e/i40e.h:179:13: note: while referencing 'list' 179 | u16 list[0]; | ^~~~ [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays Cc: Jesse Brandeburg Cc: Tony Nguyen Cc: "David S. Miller" Cc: Eric Dumazet Cc: Jakub Kicinski Cc: Paolo Abeni Cc: "Gustavo A. R. Silva" Cc: intel-wired-lan@lists.osuosl.org Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook Reviewed-by: Jiri Pirko Tested-by: Gurucharan G (A Contingent worker at Intel) Reviewed-by: Gustavo A. R. Silva Link: https://lore.kernel.org/r/20230105234557.never.799-kees@kernel.org --- drivers/net/ethernet/intel/i40e/i40e.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h index 60e351665c70..3a1c28ca5bb4 100644 --- a/drivers/net/ethernet/intel/i40e/i40e.h +++ b/drivers/net/ethernet/intel/i40e/i40e.h @@ -176,7 +176,7 @@ enum i40e_interrupt_policy { struct i40e_lump_tracking { u16 num_entries; - u16 list[0]; + u16 list[]; #define I40E_PILE_VALID_BIT 0x8000 #define I40E_IWARP_IRQ_PILE_ID (I40E_PILE_VALID_BIT - 2) }; -- cgit v1.2.3 From aa85923a954e7704bc9d3847dabeb8540aa98d13 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 5 Jan 2023 20:19:48 -0800 Subject: crypto: hisilicon: Wipe entire pool on error To work around a Clang __builtin_object_size bug that shows up under CONFIG_FORTIFY_SOURCE and UBSAN_BOUNDS, move the per-loop-iteration mem_block wipe into a single wipe of the entire pool structure after the loop. Reported-by: Nathan Chancellor Link: https://github.com/ClangBuiltLinux/linux/issues/1780 Cc: Weili Qian Cc: Zhou Wang Cc: Herbert Xu Cc: "David S. Miller" Cc: linux-crypto@vger.kernel.org Signed-off-by: Kees Cook Tested-by: Nathan Chancellor # build Link: https://lore.kernel.org/r/20230106041945.never.831-kees@kernel.org --- drivers/crypto/hisilicon/sgl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers') diff --git a/drivers/crypto/hisilicon/sgl.c b/drivers/crypto/hisilicon/sgl.c index 2b6f2281cfd6..0974b0041405 100644 --- a/drivers/crypto/hisilicon/sgl.c +++ b/drivers/crypto/hisilicon/sgl.c @@ -124,9 +124,8 @@ err_free_mem: for (j = 0; j < i; j++) { dma_free_coherent(dev, block_size, block[j].sgl, block[j].sgl_dma); - memset(block + j, 0, sizeof(*block)); } - kfree(pool); + kfree_sensitive(pool); return ERR_PTR(-ENOMEM); } EXPORT_SYMBOL_GPL(hisi_acc_create_sgl_pool); -- cgit v1.2.3