On Mon, Oct 24, 2022 at 10:20:57AM -0700, Kees Cook wrote:
> The compiler is not able to automatically perform bounds checking on
> structures that end in flexible arrays: __builtin_object_size() is
> compile-time only, and __builtin_dynamic_object_size() may not always
> be able to figure it out. Any possible run-time checks are currently
> short-circuited (or trigger false positives) because there isn't
> an obvious common way to figure out the bounds of such a structure.
> C has no way (yet[1]) to signify which struct member holds the number
> of allocated flexible array elements (like exists in other languages).
>
> As a result, the kernel (and C projects generally) need to manually
> check the bounds, check the element size calculations, and perform sanity
> checking on all the associated variable types in between (e.g. 260
> cannot be stored in a u8). This is extremely fragile.
>
> However, even if we could do all this through a magic memcpy(), the API
> itself doesn't provide meaningful feedback, which forces the kernel
> into an "all or nothing" approach: either do the copy or panic the
> system. Any failure conditions should be _detectable_, with API users
> able to gracefully recover.
>
> To deal with these needs, create the first of a set of helper functions
> that do the work of memcpy() but perform the needed bounds checking
> based on the arguments given: flex_cpy().
>
> This API will be expanded in the future to also include the common
> pattern of "allocate and copy": flex_dup(), "deserialize and copy":
> mem_to_flex(), and "deserialize, allocate, and copy": mem_to_flex_dup().
>
> The concept of a "flexible array structure" is introduced, which is a
> struct that has both a trailing flexible array member _and_ an element
> count member. If a struct lacks the element count member, it's just a
> blob: there are no bounds associated with it.
>
> The most common style of flexible array struct in the kernel is a
> "simple" one, where both the flex-array and element-count are present:
>
> struct flex_array_struct_example {
> ... /* arbitrary members */
> u16 part_count; /* count of elements stored in "parts" below. */
> ... /* arbitrary members */
> u32 parts[]; /* flexible array with elements of type u32. */
> };
>
> Next are "encapsulating flexible array structs", which is just a struct
> that contains a flexible array struct as its final member:
>
> struct encapsulating_example {
> ... /* arbitrary members */
> struct flex_array_struct_example fas;
> };
>
> There are also "split" flex array structs, which have the element-count
> member in a separate struct level than the flex-array member:
>
> struct split_example {
> ... /* arbitrary members */
> u16 part_count; /* count of elements stored in "parts" below. */
> ... /* arbitrary members */
> struct blob_example {
> ... /* other blob members */
> u32 parts[];/* flexible array with elements of type u32. */
> } blob;
> };
>
> To have the helpers deal with these arbitrary layouts, the names of the
> flex-array and element-count members need to be specified with each use
> (since C lacks the array-with-length syntax[1] so the compiler cannot
> automatically determine them). However, for the "simple" (most common)
> case, we can get close to "automatic" by explicitly declaring common
> member aliases "__flex_array_elements", and "__flex_array_elements_count"
> respectively. The regular helpers use these members, but extended helpers
> exist to cover the other two code patterns.
>
> For example, using the newly introduced flex_cpy():
>
> /* Flexible array struct with members identified. */
> struct simple {
> int mode;
> DECLARE_FAS_COUNT(int, how_many);
> unsigned long flags;
> DECLARE_FAS_ARRAY(u32, value);
> };
> ...
>
> int do_simple(struct simple *src) {
> struct simple *instance = NULL;
> size_t bytes;
> int rc;
>
> /* Allocate */
> if (fas_bytes(src, &bytes))
> return -E2BIG;
> instance = kmalloc(bytes, GFP_KERNEL);
> if (!instance)
> return -ENOMEM;
> instance->how_many = src->how_many;
> /* Copy */
> rc = flex_cpy(instance, src);
> if (rc) {
> kfree(instance);
> return rc;
> }
> return 0;
> }
>
> If anything goes wrong, it returns a negative errno. Note that the
> "allocate" pattern above will be the basis of the future flex_dup()
> helper.
>
> With these helpers the kernel can move away from many of the open-coded
> patterns of using memcpy() with a dynamically-sized destination buffer.
>
> [1] https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1990.htm
>
> Cc: "Gustavo A. R. Silva" <gustavoars@xxxxxxxxxx>
> Cc: Keith Packard <keithp@xxxxxxxxxx>
> Cc: Francis Laniel <laniel_francis@xxxxxxxxxxxxxxxxxxx>
> Cc: Daniel Axtens <dja@xxxxxxxxxx>
> Cc: Dan Williams <dan.j.williams@xxxxxxxxx>
> Cc: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
> Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> Cc: Tadeusz Struk <tadeusz.struk@xxxxxxxxxx>
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> Link: https://lore.kernel.org/r/20220504014440.3697851-3-keescook@xxxxxxxxxxxx
> ---
> include/linux/flex_array.h | 325 ++++++++++++++++++++++++++++++++++++
> include/linux/string.h | 1 +
> include/uapi/linux/stddef.h | 14 ++
> 3 files changed, 340 insertions(+)
> create mode 100644 include/linux/flex_array.h
>
> diff --git a/include/linux/flex_array.h b/include/linux/flex_array.h
> new file mode 100644
> index 000000000000..ebdbf0e5f722
> --- /dev/null
> +++ b/include/linux/flex_array.h
> @@ -0,0 +1,325 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _LINUX_FLEX_ARRAY_H_
> +#define _LINUX_FLEX_ARRAY_H_
You missed at least overflow.h and errno.h here.
> +#include <linux/string.h>
And this...
> +/* Make sure we compose correctly with KASAN. */
> +#ifndef __underlying_memcpy
> +#define __underlying_memcpy __builtin_memcpy
> +#endif
> +#ifndef __underlying_memset
> +#define __underlying_memset __builtin_memset
> +#endif
> +
> +/*
> + * A "flexible array structure" (FAS) is a structure which ends with a
> + * flexible array member (FAM) _and_ contains another member that represents
> + * how many array elements are present in the struct instance's flexible
> + * array member:
> + *
> + * struct flex_array_struct_example {
> + * ... // arbitrary members
> + * u16 part_count; // count of elements stored in "parts" below.
> + * ... // arbitrary members
> + * u32 parts[]; // flexible array member containing u32 elements.
> + * };
> + *
> + * Without the "count of elements" member, a structure ending with a
> + * flexible array has no way to check its own size, and should be
> + * considered just a blob of memory that is length-checked through some
> + * other means. Kernel structures with flexible arrays should strive to
> + * always be true flexible array structures so that they can be operated
> + * on with the flex*()-family of helpers defined below.
> + *
> + * To use the normal flex*() helpers, prepare for future C syntax that
> + * would identify a flex array's count member directly, and keep kernel
> + * declarations minimized, a common declaration, bounded_flex_array(), is
> + * provided:
> + *
> + * struct flex_array_struct_example {
> + * ... // arbitrary members
> + * bounded_flex_array(
> + * u16, part_count, // count of elements stored in "parts" below.
> + * u32, parts // flexible array with elements of type u32.
> + * );
> + * );
> + *
> + * To handle the less common case when the count member cannot be made a
> + * neighbor of the flex array, either the extended flex*() helpers can be
> + * used, or the members can also be declared separately:
> + *
> + * struct flex_array_struct_example {
> + * ... // position-sensitive members
> + * // count of elements stored in "parts" below.
> + * DECLARE_FAS_COUNT(u16, part_count);
> + * .. // position-sensitive members
> + * // flexible array with elements of type u32.
> + * DECLARE_FAS_ARRAY(u32, parts);
> + * };
> + *
> + * The above two declaration styles will create an alias for part_count as
> + * __flex_array_elements_count and for parts as __flex_array_elements,
> + * which are used internally to avoid needing to repeat the member names
> + * as arguments to the normal flex*() helpers.
> + *
> + * The extended flex*() helpers are designed to be used in the less common
> + * situations when the member aliases are not available, especially in two
> + * flexible array struct layout situations: "encapsulated" and "split".
> + *
> + * An "encapsulated flexible array structure" is a structure that contains
> + * a full "flexible array structure" as its final struct member. These are
> + * used frequently when needing to pass around a copy of a flexible array
> + * structure, and track other things about the data outside of the scope of
> + * the flexible array structure itself:
> + *
> + * struct encapsulated_example {
> + * ... // arbitrary members
> + * struct flex_array_struct_example fas;
> + * };
> + *
> + * A "split flexible array structure" is like an encapsulated flexible
> + * array struct, but the element count member is at a different level,
> + * separate from the struct that contains the flexible array:
> + *
> + * struct blob_example {
> + * ... // arbitrary members
> + * u32 parts[]; // flexible array with elements of type u32.
> + * };
> + *
> + * struct split_example {
> + * ... // arbitrary members
> + * u16 part_count; // count of elements stored in "parts" below.
> + * ... // arbitrary members
> + * struct blob_example blob;
> + * };
> + *
> + * In the case where the element count member is not stored in native
> + * CPU format, the extended helpers can be used to specify the to/from
> + * cpu helper needed to do the conversions.
> + *
> + * Examples of using flex_cpy():
> + *
> + * struct simple {
> + * u32 flags;
> + * bounded_flex_array(
> + * u32, count,
> + * u8, data
> + * );
> + * };
> + * struct hardware_defined {
> + * DECLARE_FAS_COUNT(u32, count);
> + * u32 state;
> + * u32 flags;
> + * DECLARE_FAS_ARRAY(u8, data);
> + * };
> + *
> + * int do_simple(struct simple *src) {
> + * struct simple *ptr_simple = NULL;
> + * struct hardware_defined *ptr_hw = NULL;
> + *
> + * ...allocation of ptr_simple happens...
> + * ptr_simple->count = src->count;
> + * rc = flex_cpy(ptr_simple, src);
> + * ...
> + * ...allocation of ptr_hw happens...
> + * ptr_hw->count = src->count;
> + * rc = flex_cpy(ptr_hw, src);
> + * ...
> + * }
> + *
> + * struct blob {
> + * u32 flags;
> + * u8 data[];
> + * };
> + * struct split {
> + * be32 count;
> + * struct blob blob;
> + * };
> + * struct split *ptr_split = NULL;
> + *
> + * int do_split(struct split *src) {
> + * struct split *ptr = NULL;
> + *
> + * ...allocation of ptr happens...
> + * ptr->count = src->count;
> + * rc = __flex_cpy(ptr, src, blob.data, count, be32_to_cpu);
> + * ...
> + * }
> + *
> + */
> +
> +/* Wrappers around the UAPI macros. */
> +#define bounded_flex_array(COUNT_TYPE, COUNT_NAME, ARRAY_TYPE, ARRAY_NAME) \
> + __DECLARE_FAS_COUNT(COUNT_TYPE, COUNT_NAME); \
> + __DECLARE_FAS_ARRAY(ARRAY_TYPE, ARRAY_NAME)
> +
> +#define DECLARE_FAS_COUNT(TYPE, NAME) \
> + __DECLARE_FAS_COUNT(TYPE, NAME)
> +
> +#define DECLARE_FAS_ARRAY(TYPE, NAME) \
> + __DECLARE_FAS_ARRAY(TYPE, NAME)
> +
> +/* All the helpers return negative on failure, and must be checked. */
> +static inline int __must_check __must_check_errno(int err)
> +{
> + return err;
> +}
> +
> +#define __passthru(x) (x)
> +
> +/**
> + * __fas_elements_bytes - Calculate potential size of the flexible
> + * array elements of a given flexible array
> + * structure
> + *
> + * @p: Pointer to flexible array structure.
> + * @flex_member: Member name of the flexible array elements.
> + * @count_member: Member name of the flexible array elements count.
> + * @elements_count: Count of proposed number of @p->__flex_array_elements
> + * @bytes: Pointer to variable to write calculation of total size in bytes.
> + *
> + * Returns: 0 on successful calculation, -ve on error.
> + *
> + * This performs the same calculation as flex_array_size(), except
> + * that the result is bounds checked and written to @bytes instead
> + * of being returned.
> + */
> +#define __fas_elements_bytes(p, flex_member, count_member, \
> + elements_count, bytes) \
> +__must_check_errno(({ \
> + int __feb_err = -EINVAL; \
> + size_t __feb_elements_count = (elements_count); \
> + size_t __feb_elements_max = \
> + type_max(typeof((p)->count_member)); \
> + if (__feb_elements_count > __feb_elements_max || \
> + check_mul_overflow(sizeof(*(p)->flex_member), \
> + __feb_elements_count, bytes)) { \
> + *(bytes) = 0; \
> + __feb_err = -E2BIG; \
> + } else { \
> + __feb_err = 0; \
> + } \
> + __feb_err; \
> +}))
> +
> +/**
> + * fas_elements_bytes - Calculate current size of the flexible array
> + * elements of a given flexible array structure
> + *
> + * @p: Pointer to flexible array structure.
> + * @bytes: Pointer to variable to write calculation of total size in bytes.
> + *
> + * Returns: 0 on successful calculation, -ve on error.
> + *
> + * This performs the same calculation as flex_array_size(), except
> + * that the result is bounds checked and written to @bytes instead
> + * of being returned.
> + */
> +#define fas_elements_bytes(p, bytes) \
> + __fas_elements_bytes(p, __flex_array_elements, \
> + __flex_array_elements_count, \
> + (p)->__flex_array_elements_count, bytes)
> +
> +/**
> + * __fas_bytes - Calculate potential size of flexible array structure
> + *
> + * @p: Pointer to flexible array structure.
> + * @flex_member: Member name of the flexible array elements.
> + * @count_member: Member name of the flexible array elements count.
> + * @elements_count: Count of proposed number of @p->__flex_array_elements
> + * @bytes: Pointer to variable to write calculation of total size in bytes.
> + *
> + * Returns: 0 on successful calculation, -ve on error.
> + *
> + * This performs the same calculation as struct_size(), except
> + * that the result is bounds checked and written to @bytes instead
> + * of being returned.
> + */
> +#define __fas_bytes(p, flex_member, count_member, elements_count, bytes)\
> +__must_check_errno(({ \
> + int __fasb_err; \
> + typeof(*bytes) __fasb_bytes; \
> + \
> + if (__fas_elements_bytes(p, flex_member, count_member, \
> + elements_count, &__fasb_bytes) || \
> + check_add_overflow(sizeof(*(p)), __fasb_bytes, bytes)) { \
> + *(bytes) = 0; \
> + __fasb_err = -E2BIG; \
> + } else { \
> + __fasb_err = 0; \
> + } \
> + __fasb_err; \
> +}))
> +
> +/**
> + * fas_bytes - Calculate current size of flexible array structure
> + *
> + * @p: Pointer to flexible array structure.
> + * @bytes: Pointer to variable to write calculation of total size in bytes.
> + *
> + * This performs the same calculation as struct_size(), except
> + * that the result is bounds checked and written to @bytes instead
> + * of being returned, using the current size of the flexible array
> + * structure (via @p->__flexible_array_elements_count).
> + *
> + * Returns: 0 on successful calculation, -ve on error.
> + */
> +#define fas_bytes(p, bytes) \
> + __fas_bytes(p, __flex_array_elements, \
> + __flex_array_elements_count, \
> + (p)->__flex_array_elements_count, bytes)
> +
> +/**
> + * flex_cpy - Copy from one flexible array struct into another
> + *
> + * @dst: Destination pointer
> + * @src: Source pointer
> + *
> + * The full structure of @src will be copied to @dst, including all trailing
> + * flexible array elements. @dst->__flex_array_elements_count must be large
> + * enough to hold @src->__flex_array_elements_count. Any elements left over
> + * in @dst will be zero-wiped.
> + *
> + * Returns: 0 on successful calculation, -ve on error.
> + */
> +#define __flex_cpy(dst, src, elements_member, count_member, to_cpu) \
> +__must_check_errno(({ \
> + int __fc_err = -EINVAL; \
> + typeof(*(dst)) *__fc_dst = (dst); \
> + typeof(*(src)) *__fc_src = (src); \
> + size_t __fc_dst_bytes, __fc_src_bytes; \
> + \
> + BUILD_BUG_ON(!__same_type(*(__fc_dst), *(__fc_src))); \
> + \
> + do { \
> + if (__fas_bytes(__fc_dst, \
> + elements_member, \
> + count_member, \
> + to_cpu(__fc_dst->count_member), \
> + &__fc_dst_bytes) || \
> + __fas_bytes(__fc_src, \
> + elements_member, \
> + count_member, \
> + to_cpu(__fc_src->count_member), \
> + &__fc_src_bytes) || \
> + __fc_dst_bytes < __fc_src_bytes) { \
> + /* do we need to wipe dst here? */ \
> + __fc_err = -E2BIG; \
> + break; \
> + } \
> + __underlying_memcpy(__fc_dst, __fc_src, __fc_src_bytes);\
> + /* __flex_array_elements_count is included in memcpy */ \
> + /* Wipe any now-unused trailing elements in @dst: */ \
> + __underlying_memset((u8 *)__fc_dst + __fc_src_bytes, 0, \
> + __fc_dst_bytes - __fc_src_bytes); \
> + __fc_err = 0; \
> + } while (0); \
> + __fc_err; \
> +}))
> +
> +#define flex_cpy(dst, src) \
> + __flex_cpy(dst, src, __flex_array_elements, \
> + __flex_array_elements_count, __passthru)
> +
> +#endif /* _LINUX_FLEX_ARRAY_H_ */
> diff --git a/include/linux/string.h b/include/linux/string.h
> index cf7607b32102..9277f9e2a432 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -256,6 +256,7 @@ static inline const char *kbasename(const char *path)
> #define unsafe_memcpy(dst, src, bytes, justification) \
> memcpy(dst, src, bytes)
> #endif
> +#include <linux/flex_array.h>
...seems to be a hidden mine for the future. Can we avoid dependency hell, please?
> void memcpy_and_pad(void *dest, size_t dest_len, const void *src, size_t count,
> int pad);
> diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h
> index 7837ba4fe728..e16afe1951d8 100644
> --- a/include/uapi/linux/stddef.h
> +++ b/include/uapi/linux/stddef.h
> @@ -44,4 +44,18 @@
> struct { } __empty_ ## NAME; \
> TYPE NAME[]; \
> }
> +
> +/* For use with flexible array structure helpers, in <linux/flex_array.h> */
> +#define __DECLARE_FAS_COUNT(TYPE, NAME) \
> + union { \
> + TYPE __flex_array_elements_count; \
> + TYPE NAME; \
> + }
> +
> +#define __DECLARE_FAS_ARRAY(TYPE, NAME) \
> + union { \
> + __DECLARE_FLEX_ARRAY(TYPE, __flex_array_elements); \
> + __DECLARE_FLEX_ARRAY(TYPE, NAME); \
> + }
> +
> #endif
> --
> 2.34.1
>
--
With Best Regards,
Andy Shevchenko
