diff mbox series

[v4,2/8] overflow: Add struct_size_with_data() and struct_data_pointer() helpers

Message ID 20240228204919.3680786-3-andriy.shevchenko@linux.intel.com (mailing list archive)
State Not Applicable
Headers show
Series iio: core: New macros and making use of them | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 15541 this patch: 15541
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 3 of 3 maintainers
netdev/build_clang success Errors and warnings before: 3127 this patch: 3127
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 16681 this patch: 16681
netdev/checkpatch warning WARNING: line length of 84 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Andy Shevchenko Feb. 28, 2024, 8:41 p.m. UTC
Introduce two helper macros to calculate the size of the structure
with trailing aligned data and to retrieve the pointer to that data.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/overflow.h | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

Comments

Kees Cook Feb. 28, 2024, 9:37 p.m. UTC | #1
On Wed, Feb 28, 2024 at 10:41:32PM +0200, Andy Shevchenko wrote:
> Introduce two helper macros to calculate the size of the structure
> with trailing aligned data and to retrieve the pointer to that data.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/overflow.h | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/overflow.h b/include/linux/overflow.h
> index bc390f026128..b93bbf1b6aaa 100644
> --- a/include/linux/overflow.h
> +++ b/include/linux/overflow.h
> @@ -2,9 +2,10 @@
>  #ifndef __LINUX_OVERFLOW_H
>  #define __LINUX_OVERFLOW_H
>  
> +#include <linux/align.h>
>  #include <linux/compiler.h>
> -#include <linux/limits.h>
>  #include <linux/const.h>
> +#include <linux/limits.h>
>  
>  /*
>   * We need to compute the minimum and maximum values representable in a given
> @@ -337,6 +338,30 @@ static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
>   */
>  #define array3_size(a, b, c)	size_mul(size_mul(a, b), c)
>  
> +/**
> + * struct_size_with_data() - Calculate size of structure with trailing aligned data.
> + * @p: Pointer to the structure.
> + * @a: Alignment in bytes before trailing data.
> + * @s: Data size in bytes (must not be 0).
> + *
> + * Calculates size of memory needed for structure of @p followed by an
> + * aligned data of size @s.
> + *
> + * Return: number of bytes needed or SIZE_MAX on overflow.
> + */
> +#define struct_size_with_data(p, a, s)	size_add(ALIGN(sizeof(*(p)), (a)), (s))

I don't like this -- "p" should have a trailing flexible array. (See
below.)

> +
> +/**
> + * struct_data_pointer - Calculate offset of the trailing data reserved with
> + * struct_size_with_data().
> + * @p: Pointer to the structure.
> + * @a: Alignment in bytes before trailing data.
> + *
> + * Return: offset in bytes to the trailing data reserved with
> + * struct_size_with_data().
> + */
> +#define struct_data_pointer(p, a)	PTR_ALIGN((void *)((p) + 1), (a))

I'm not super excited about propagating the "p + 1" code pattern to find
things after an allocation. This leads to the compiler either being
blind to accesses beyond an allocation, or being too conservative about
accesses beyond an object. Instead of these helpers I would much prefer
that data structures that use this code pattern be converted to using
trailing flexible arrays, at which point the compiler is in a much
better position to reason about sizes.
Andy Shevchenko Feb. 28, 2024, 9:51 p.m. UTC | #2
On Wed, Feb 28, 2024 at 01:37:36PM -0800, Kees Cook wrote:
> On Wed, Feb 28, 2024 at 10:41:32PM +0200, Andy Shevchenko wrote:

...

> > +#define struct_data_pointer(p, a)	PTR_ALIGN((void *)((p) + 1), (a))
> 
> I'm not super excited about propagating the "p + 1" code pattern to find
> things after an allocation. This leads to the compiler either being
> blind to accesses beyond an allocation, or being too conservative about
> accesses beyond an object. Instead of these helpers I would much prefer
> that data structures that use this code pattern be converted to using
> trailing flexible arrays, at which point the compiler is in a much
> better position to reason about sizes.

There is nothing about flexible arrays in this.
Maybe you have been confused by my choice for name of the macros.
In that case I also can argue that current struct_size() is a good one.
(something like struct_size_with_flex_array() can be more specific)
diff mbox series

Patch

diff --git a/include/linux/overflow.h b/include/linux/overflow.h
index bc390f026128..b93bbf1b6aaa 100644
--- a/include/linux/overflow.h
+++ b/include/linux/overflow.h
@@ -2,9 +2,10 @@ 
 #ifndef __LINUX_OVERFLOW_H
 #define __LINUX_OVERFLOW_H
 
+#include <linux/align.h>
 #include <linux/compiler.h>
-#include <linux/limits.h>
 #include <linux/const.h>
+#include <linux/limits.h>
 
 /*
  * We need to compute the minimum and maximum values representable in a given
@@ -337,6 +338,30 @@  static inline size_t __must_check size_sub(size_t minuend, size_t subtrahend)
  */
 #define array3_size(a, b, c)	size_mul(size_mul(a, b), c)
 
+/**
+ * struct_size_with_data() - Calculate size of structure with trailing aligned data.
+ * @p: Pointer to the structure.
+ * @a: Alignment in bytes before trailing data.
+ * @s: Data size in bytes (must not be 0).
+ *
+ * Calculates size of memory needed for structure of @p followed by an
+ * aligned data of size @s.
+ *
+ * Return: number of bytes needed or SIZE_MAX on overflow.
+ */
+#define struct_size_with_data(p, a, s)	size_add(ALIGN(sizeof(*(p)), (a)), (s))
+
+/**
+ * struct_data_pointer - Calculate offset of the trailing data reserved with
+ * struct_size_with_data().
+ * @p: Pointer to the structure.
+ * @a: Alignment in bytes before trailing data.
+ *
+ * Return: offset in bytes to the trailing data reserved with
+ * struct_size_with_data().
+ */
+#define struct_data_pointer(p, a)	PTR_ALIGN((void *)((p) + 1), (a))
+
 /**
  * flex_array_size() - Calculate size of a flexible array member
  *                     within an enclosing structure.