From 6a6b3bdb13d51ccb865cfa56ad61533dd432f722 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Thu, 17 Aug 2023 12:21:20 +0200 Subject: overflow: Implement struct_size() in terms of offsetof() When a structure with a flex array has trailing padding, the trailing padding may overlap with the flex array. This means that sizeof(s) + sizeof_field(s, fam) may give a value higher than the actual size of the structure. The correct way to get the exact size is with offsetof(): MAX(sizeof(s), offsetof(s, fam), sizeof(s->fam[0])); We need MAX(sizeof(s), ...) because the offsetof() calculation doesn't take into account the padding, so if the size of the FAM is smaller than the padding, we would be returning a size that is smaller than the actual size of the structure. The old way of calculating the size wasn't so bad; it would be just wasting a few bytes in few cases. But since these calculations are constexpr most of the time, it's basically free to use the exact size. I was more worried about something different: a programmer may be tricked to think that using sizeof() of a structure with a flex array is good. It's (almost?) always a bad idea, with the only exception being the implementation of struct_size(), within MAX(). If sizeof() is used in other pieces of code, the bug is likely to not be as benign as here, and instead of wasting a few bytes, the bug will manifest as reading or writing a few bytes off. See the link below for a discussion in GCC about adding a warning for those cases. Being pedantically correct here will serve as an example. Link: Cc: Kees Cook Cc: Gustavo A. R. Silva Cc: Signed-off-by: Alejandro Colomar --- include/linux/overflow.h | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/include/linux/overflow.h b/include/linux/overflow.h index 0e33b5cbdb9f..9d3fffc39bec 100644 --- a/include/linux/overflow.h +++ b/include/linux/overflow.h @@ -289,8 +289,11 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend) * Return: number of bytes needed or SIZE_MAX on overflow. */ #define struct_size(p, member, count) \ - __builtin_choose_expr(__is_constexpr(count), \ - sizeof(*(p)) + flex_array_size(p, member, count), \ - size_add(sizeof(*(p)), flex_array_size(p, member, count))) + max(sizeof(*(p)), \ + __builtin_choose_expr(__is_constexpr(count), \ + offsetof(typeof(*(p)), member) + \ + flex_array_size(p, member, count), \ + size_add(offsetof(typeof(*(p)), member), \ + flex_array_size(p, member, count)))) #endif /* __LINUX_OVERFLOW_H */ -- cgit v1.2.3