From d77008421afda6208b1256c9b218457acd174ca6 Mon Sep 17 00:00:00 2001 From: Elena Reshetova Date: Thu, 17 Aug 2023 21:14:57 -0700 Subject: groups: Convert group_info.usage to refcount_t atomic_t variables are currently used to implement reference counters with the following properties: - counter is initialized to 1 using atomic_set() - a resource is freed upon counter reaching zero - once counter reaches zero, its further increments aren't allowed - counter schema uses basic atomic operations (set, inc, inc_not_zero, dec_and_test, etc.) Such atomic variables should be converted to a newly provided refcount_t type and API that prevents accidental counter overflows and underflows. This is important since overflows and underflows can lead to use-after-free situation and be exploitable. The variable group_info.usage is used as pure reference counter. Convert it to refcount_t and fix up the operations. **Important note for maintainers: Some functions from refcount_t API defined in refcount.h have different memory ordering guarantees than their atomic counterparts. Please check Documentation/core-api/refcount-vs-atomic.rst for more information. Normally the differences should not matter since refcount_t provides enough guarantees to satisfy the refcounting use cases, but in some rare cases it might matter. Please double check that you don't have some undocumented memory guarantees for this variable usage. For the group_info.usage it might make a difference in following places: - put_group_info(): decrement in refcount_dec_and_test() only provides RELEASE ordering and ACQUIRE ordering on success vs. fully ordered atomic counterpart Suggested-by: Kees Cook Signed-off-by: Elena Reshetova Reviewed-by: David Windsor Reviewed-by: Hans Liljestrand Link: https://lore.kernel.org/r/20230818041456.gonna.009-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/cred.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'include/linux') diff --git a/include/linux/cred.h b/include/linux/cred.h index f923528d5cc4..92f8d772da6f 100644 --- a/include/linux/cred.h +++ b/include/linux/cred.h @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -23,7 +24,7 @@ struct inode; * COW Supplementary groups list */ struct group_info { - atomic_t usage; + refcount_t usage; int ngroups; kgid_t gid[]; } __randomize_layout; @@ -39,7 +40,7 @@ struct group_info { */ static inline struct group_info *get_group_info(struct group_info *gi) { - atomic_inc(&gi->usage); + refcount_inc(&gi->usage); return gi; } @@ -49,7 +50,7 @@ static inline struct group_info *get_group_info(struct group_info *gi) */ #define put_group_info(group_info) \ do { \ - if (atomic_dec_and_test(&(group_info)->usage)) \ + if (refcount_dec_and_test(&(group_info)->usage)) \ groups_free(group_info); \ } while (0) -- cgit v1.2.3 From 5234193ee2b997e59326b047610a8f3f64a0ce02 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 15 Sep 2023 13:15:17 -0700 Subject: ceph: Annotate struct ceph_osd_request with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct ceph_osd_request. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Ilya Dryomov Cc: Xiubo Li Cc: Jeff Layton Cc: ceph-devel@vger.kernel.org Reviewed-by: "Gustavo A. R. Silva" Reviewed-by: Xiubo Li Link: https://lore.kernel.org/r/20230915201517.never.373-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/ceph/osd_client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/ceph/osd_client.h b/include/linux/ceph/osd_client.h index bf9823956758..b8610e9d2471 100644 --- a/include/linux/ceph/osd_client.h +++ b/include/linux/ceph/osd_client.h @@ -278,7 +278,7 @@ struct ceph_osd_request { int r_attempts; u32 r_map_dne_bound; - struct ceph_osd_req_op r_ops[]; + struct ceph_osd_req_op r_ops[] __counted_by(r_num_ops); }; struct ceph_request_redirect { -- cgit v1.2.3 From 4a530cb932af31b0c919a109bc107dd186653381 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:50:53 -0700 Subject: hwmon: Annotate struct gsc_hwmon_platform_data with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct gsc_hwmon_platform_data. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Tim Harvey Reviewed-by: "Gustavo A. R. Silva" Link: https://lore.kernel.org/r/20230922175053.work.564-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/platform_data/gsc_hwmon.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/platform_data/gsc_hwmon.h b/include/linux/platform_data/gsc_hwmon.h index f2781aa7eff8..70e8a6bec0f6 100644 --- a/include/linux/platform_data/gsc_hwmon.h +++ b/include/linux/platform_data/gsc_hwmon.h @@ -40,6 +40,6 @@ struct gsc_hwmon_platform_data { unsigned int resolution; unsigned int vreference; unsigned int fan_base; - struct gsc_hwmon_channel channels[]; + struct gsc_hwmon_channel channels[] __counted_by(nchannels); }; #endif -- cgit v1.2.3 From a48e1f656b3c9b8192b6ca6fc92ef4daa30535fb Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:51:21 -0700 Subject: KVM: Annotate struct kvm_irq_routing_table with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct kvm_irq_routing_table. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Paolo Bonzini Cc: kvm@vger.kernel.org Reviewed-by: "Gustavo A. R. Silva" Link: https://lore.kernel.org/r/20230922175121.work.660-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/kvm_host.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index fb6c6109fdca..4944136efaa2 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -664,7 +664,7 @@ struct kvm_irq_routing_table { * Array indexed by gsi. Each entry contains list of irq chips * the gsi is connected to. */ - struct hlist_head map[]; + struct hlist_head map[] __counted_by(nr_rt_entries); }; #endif -- cgit v1.2.3 From 0e108725f6cc5b3be9e607f89c9fbcbb236367b7 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Wed, 18 Oct 2023 10:53:58 -0700 Subject: 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 Fixes: dfbafa70bde2 ("string: Introduce strtomem() and strtomem_pad()") Tested-by: Arnd Bergmann Cc: Andy Shevchenko Cc: linux-hardening@vger.kernel.org Signed-off-by: Kees Cook --- include/linux/string.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'include/linux') diff --git a/include/linux/string.h b/include/linux/string.h index dbfc66400050..9e3cb6923b0e 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -277,10 +277,12 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, */ #define strtomem_pad(dest, src, pad) do { \ const size_t _dest_len = __builtin_object_size(dest, 1); \ + const size_t _src_len = __builtin_object_size(src, 1); \ \ BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ _dest_len == (size_t)-1); \ - memcpy_and_pad(dest, _dest_len, src, strnlen(src, _dest_len), pad); \ + memcpy_and_pad(dest, _dest_len, src, \ + strnlen(src, min(_src_len, _dest_len)), pad); \ } while (0) /** @@ -298,10 +300,11 @@ void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count, */ #define strtomem(dest, src) do { \ const size_t _dest_len = __builtin_object_size(dest, 1); \ + const size_t _src_len = __builtin_object_size(src, 1); \ \ BUILD_BUG_ON(!__builtin_constant_p(_dest_len) || \ _dest_len == (size_t)-1); \ - memcpy(dest, src, min(_dest_len, strnlen(src, _dest_len))); \ + memcpy(dest, src, strnlen(src, min(_src_len, _dest_len))); \ } while (0) /** -- cgit v1.2.3 From 15fcedd43a0810a5482ef759a18ab852d012cead Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 22 Sep 2023 10:52:24 -0700 Subject: kexec: Annotate struct crash_mem with __counted_by Prepare for the coming implementation by GCC and Clang of the __counted_by attribute. Flexible array members annotated with __counted_by can have their accesses bounds-checked at run-time checking via CONFIG_UBSAN_BOUNDS (for array indexing) and CONFIG_FORTIFY_SOURCE (for strcpy/memcpy-family functions). As found with Coccinelle[1], add __counted_by for struct crash_mem. [1] https://github.com/kees/kernel-tools/blob/trunk/coccinelle/examples/counted_by.cocci Cc: Eric Biederman Cc: kexec@lists.infradead.org Acked-by: Baoquan He Link: https://lore.kernel.org/r/20230922175224.work.712-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/crash_core.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/crash_core.h b/include/linux/crash_core.h index 0c06561bf5ff..08704c29fdb4 100644 --- a/include/linux/crash_core.h +++ b/include/linux/crash_core.h @@ -92,7 +92,7 @@ int parse_crashkernel_low(char *cmdline, unsigned long long system_ram, struct crash_mem { unsigned int max_nr_ranges; unsigned int nr_ranges; - struct range ranges[]; + struct range ranges[] __counted_by(max_nr_ranges); }; extern int crash_exclude_mem_range(struct crash_mem *mem, -- cgit v1.2.3