diff mbox series

[02/32] Introduce flexible array struct memcpy() helpers

Message ID 20220504014440.3697851-3-keescook@chromium.org (mailing list archive)
State Not Applicable
Delegated to: Netdev Maintainers
Headers show
Series Introduce flexible array struct memcpy() helpers | expand

Checks

Context Check Description
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count fail Series longer than 15 patches (and no cover letter)
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18181 this patch: 18181
netdev/cc_maintainers warning 1 maintainers not CCed: andriy.shevchenko@linux.intel.com
netdev/build_clang success Errors and warnings before: 3315 this patch: 3315
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 17416 this patch: 17416
netdev/checkpatch warning CHECK: Macro argument 'bytes' may be better as '(bytes)' to avoid precedence issues CHECK: Macro argument 'count_member' may be better as '(count_member)' to avoid precedence issues CHECK: Macro argument 'flex_member' may be better as '(flex_member)' to avoid precedence issues CHECK: spaces preferred around that '*' (ctx:WxV) WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: line length of 85 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/tree_selection success Guessing tree name failed - patch did not apply

Commit Message

Kees Cook May 4, 2022, 1:44 a.m. UTC
The compiler is not able to automatically perform bounds checking
on structures that end in flexible arrays: __builtin_object_size()
is compile-time only. Any possible run-time checks are currently
short-circuited 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 a set of helper functions that do the
work of memcpy() but perform the needed bounds checking based on the
arguments given: flex_cpy(). The common pattern of "allocate and copy"
is also included: flex_dup(). However, one of the most common patterns
is deserialization: allocating and populating flexible array members
from a byte array: mem_to_flex_dup(). And if the elements are already
allocated: mem_to_flex().

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
"normal" 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 "normal" (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 most complicated helper, mem_to_flex_dup():

    /* Flexible array struct with members identified. */
    struct something {
        int mode;
        DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(int, how_many);
        unsigned long flags;
        DECLARE_FLEX_ARRAY_ELEMENTS(u32, value);
    };
    ...
    struct something *instance = NULL;
    int rc;

    rc = mem_to_flex_dup(&instance, byte_array, count, GFP_KERNEL);
    if (rc)
        return rc;

This will:

- validate "instance" is non-NULL (no NULL dereference).
- validate "*instance" is NULL (no memory allocation resource leak).
- validate that "count" is:
  - non-negative (no arithmetic underflow).
  - has a value that can be stored in the "how_many" type (no value
    truncation).
- calculate the bytes needed to store "count"-many trailing u32 elements
  (no arithmetic overflow/underflow).
- calculate the bytes needed for a "struct something" with the above
  trailing elements (no arithmetic overflow/underflow).
- allocate the memory and check the result (no NULL dereference).
- initialize the non-flex-array portion of the struct to zero (no
  uninitialized memory usage).
- copy from "buf" into the flexible array elements.

If anything goes wrong, it returns a negative errno.

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@kernel.org>
Cc: Keith Packard <keithp@keithp.com>
Cc: Francis Laniel <laniel_francis@privacyrequired.com>
Cc: Daniel Axtens <dja@axtens.net>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Tadeusz Struk <tadeusz.struk@linaro.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/flex_array.h  | 637 ++++++++++++++++++++++++++++++++++++
 include/linux/string.h      |   1 +
 include/uapi/linux/stddef.h |  14 +
 3 files changed, 652 insertions(+)
 create mode 100644 include/linux/flex_array.h

Comments

Johannes Berg May 4, 2022, 7:25 a.m. UTC | #1
On Tue, 2022-05-03 at 18:44 -0700, Kees Cook wrote:
> 
> For example, using the most complicated helper, mem_to_flex_dup():
> 
>     /* Flexible array struct with members identified. */
>     struct something {
>         int mode;
>         DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(int, how_many);
>         unsigned long flags;
>         DECLARE_FLEX_ARRAY_ELEMENTS(u32, value);

In many cases, the order of the elements doesn't really matter, so maybe
it'd be nicer to be able to write it as something like

DECLARE_FLEX_STRUCT(something,
	int mode;
	unsigned long flags;
	,
	int, how_many,
	u32, value);

perhaps? OK, that doesn't seem so nice either.

Maybe

struct something {
	int mode;
	unsigned long flags;
	FLEX_ARRAY(
		int, how_many,
		u32, value
	);
};

or so? The long and duplicated DECLARE_FLEX_ARRAY_ELEMENTS_COUNT and
DECLARE_FLEX_ARRAY_ELEMENTS seems a bit tedious to me, at least in cases
where the struct layout is not the most important thing (or it's already
at the end anyway).


>     struct something *instance = NULL;
>     int rc;
> 
>     rc = mem_to_flex_dup(&instance, byte_array, count, GFP_KERNEL);
>     if (rc)
>         return rc;

This seems rather awkward, having to set it to NULL, then checking rc
(and possibly needing a separate variable for it), etc.

But I can understand how you arrived at this:
 - need to pass instance or &instance or such for typeof()
   or offsetof() or such
 - instance = mem_to_flex_dup(instance, ...)
   looks too much like it would actually dup 'instance', rather than
   'byte_array'
 - if you pass &instance anyway, checking for NULL is simple and adds a
   bit of safety

but still, honestly, I don't like it. As APIs go, it feels a bit
cumbersome and awkward to use, and you really need everyone to use this,
and not say "uh what, I'll memcpy() instead".

Maybe there should also be a realloc() version of it?


> +/** __fas_bytes - Calculate potential size of flexible array structure

I think you forgot "\n *" in many cases here after "/**".

johannes
Kees Cook May 4, 2022, 3:38 p.m. UTC | #2
On Wed, May 04, 2022 at 09:25:56AM +0200, Johannes Berg wrote:
> On Tue, 2022-05-03 at 18:44 -0700, Kees Cook wrote:
> > 
> > For example, using the most complicated helper, mem_to_flex_dup():
> > 
> >     /* Flexible array struct with members identified. */
> >     struct something {
> >         int mode;
> >         DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(int, how_many);
> >         unsigned long flags;
> >         DECLARE_FLEX_ARRAY_ELEMENTS(u32, value);
> 
> In many cases, the order of the elements doesn't really matter, so maybe
> it'd be nicer to be able to write it as something like
> 
> DECLARE_FLEX_STRUCT(something,
> 	int mode;
> 	unsigned long flags;
> 	,
> 	int, how_many,
> 	u32, value);
> 
> perhaps? OK, that doesn't seem so nice either.
> 
> Maybe
> 
> struct something {
> 	int mode;
> 	unsigned long flags;
> 	FLEX_ARRAY(
> 		int, how_many,
> 		u32, value
> 	);
> };

Yeah, I mention some of my exploration of this idea in the sibling reply:
https://lore.kernel.org/linux-hardening/202205040730.161645EC@keescook/#t

It seemed like requiring a structure be rearranged to take advantage of
the "automatic layout introspection" wasn't very friendly. On the other
hand, looking at the examples, most of them are already neighboring
members. Hmmm.

> or so? The long and duplicated DECLARE_FLEX_ARRAY_ELEMENTS_COUNT and
> DECLARE_FLEX_ARRAY_ELEMENTS seems a bit tedious to me, at least in cases
> where the struct layout is not the most important thing (or it's already
> at the end anyway).

The names aren't great, but I wanted to distinguish "elements" as the
array not the count. Yay naming.

However, perhaps the solution is to have _both_. i.e using
BOUNDED_FLEX_ARRAY(count_type, count_name, array_type, array_name) for
the "neighboring" case, and the DECLARE...{ELEMENTS,COUNT} for the
"split" case.

And DECLARE_FLEX_ARRAY_ELEMENTS could actually be expanded to include
the count_name too, so both methods could be "forward portable" to a
future where C grew the syntax for bounded flex arrays.

> 
> >     struct something *instance = NULL;
> >     int rc;
> > 
> >     rc = mem_to_flex_dup(&instance, byte_array, count, GFP_KERNEL);
> >     if (rc)
> >         return rc;
> 
> This seems rather awkward, having to set it to NULL, then checking rc
> (and possibly needing a separate variable for it), etc.

I think the errno return is completely required. I had an earlier version
of this that was much more like a drop-in replacement for memcpy that
would just truncate or panic, and when I had it all together, I could
just imagine hearing Linus telling me to start over because it was unsafe
(truncation may be just as bad as overflow) and disruptive ("never BUG"),
and that it should be recoverable. So, I rewrote it all to return a
__must_check errno.

Requiring instance to be NULL is debatable, but I feel pretty strongly
about it because it does handle a class of mistakes (resource leaks),
and it's not much of a burden to require a known-good starting state.

> But I can understand how you arrived at this:
>  - need to pass instance or &instance or such for typeof()
>    or offsetof() or such

Right.

>  - instance = mem_to_flex_dup(instance, ...)
>    looks too much like it would actually dup 'instance', rather than
>    'byte_array'

And I need an errno output to keep imaginary Linus happy. :)

>  - if you pass &instance anyway, checking for NULL is simple and adds a
>    bit of safety

Right.

> but still, honestly, I don't like it. As APIs go, it feels a bit
> cumbersome and awkward to use, and you really need everyone to use this,
> and not say "uh what, I'll memcpy() instead".

Sure, and I have tried to get it down as small as possible. The earlier
"just put all the member names in every call" version was horrid. :P I
realize it's more work to check errno, but the memcpy() API we've all
been trained to use is just plain dangerous. I don't think it's
unreasonable to ask people to retrain themselves to avoid it. All that
said, yes, I want it to be as friendly as possible.

> Maybe there should also be a realloc() version of it?

Sure! Seems reasonable. I'd like to see the code pattern for this
though. Do you have any examples? Most of what I'd been able to find for
the fragile memcpy() usage was just basic serialize/deserialize or
direct copying.

> > +/** __fas_bytes - Calculate potential size of flexible array structure
> 
> I think you forgot "\n *" in many cases here after "/**".

Oops! Yes, thank you. I'll fix these.

-Kees
David Laight May 4, 2022, 4:08 p.m. UTC | #3
From: Kees Cook
> Sent: 04 May 2022 16:38
...
> > >     struct something *instance = NULL;
> > >     int rc;
> > >
> > >     rc = mem_to_flex_dup(&instance, byte_array, count, GFP_KERNEL);
> > >     if (rc)
> > >         return rc;
> >
> > This seems rather awkward, having to set it to NULL, then checking rc
> > (and possibly needing a separate variable for it), etc.
> 
> I think the errno return is completely required. I had an earlier version
> of this that was much more like a drop-in replacement for memcpy that
> would just truncate or panic, and when I had it all together, I could
> just imagine hearing Linus telling me to start over because it was unsafe
> (truncation may be just as bad as overflow) and disruptive ("never BUG"),
> and that it should be recoverable. So, I rewrote it all to return a
> __must_check errno.
> 
> Requiring instance to be NULL is debatable, but I feel pretty strongly
> about it because it does handle a class of mistakes (resource leaks),
> and it's not much of a burden to require a known-good starting state.

Why not make it look like malloc() since it seems to be malloc().
That gives a much better calling convention.
Passing pointers and integers by reference can generate horrid code.
(Mostly because it stops the compiler keeping values in registers.)

If you want the type information inside the 'function'
use a #define so that the use is:

	mem_to_flex_dup(instance, byte_array, count, GFP_KERNEL);
	if (!instance)
		return ...
(or use ERR_PTR() etc).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Johannes Berg May 5, 2022, 1:16 p.m. UTC | #4
On Wed, 2022-05-04 at 08:38 -0700, Kees Cook wrote:
> 
> It seemed like requiring a structure be rearranged to take advantage of
> the "automatic layout introspection" wasn't very friendly. On the other
> hand, looking at the examples, most of them are already neighboring
> members. Hmmm.

A lot of them are, and many could be, though not all.

> > or so? The long and duplicated DECLARE_FLEX_ARRAY_ELEMENTS_COUNT and
> > DECLARE_FLEX_ARRAY_ELEMENTS seems a bit tedious to me, at least in cases
> > where the struct layout is not the most important thing (or it's already
> > at the end anyway).
> 
> The names aren't great, but I wanted to distinguish "elements" as the
> array not the count. Yay naming.

:-)

> However, perhaps the solution is to have _both_. i.e using
> BOUNDED_FLEX_ARRAY(count_type, count_name, array_type, array_name) for
> the "neighboring" case, and the DECLARE...{ELEMENTS,COUNT} for the
> "split" case.

Seems reasonable to me.

> And DECLARE_FLEX_ARRAY_ELEMENTS could actually be expanded to include
> the count_name too, so both methods could be "forward portable" to a
> future where C grew the syntax for bounded flex arrays.

I guess I don't see that happening :)

> > This seems rather awkward, having to set it to NULL, then checking rc
> > (and possibly needing a separate variable for it), etc.
> 
> I think the errno return is completely required. I had an earlier version
> of this that was much more like a drop-in replacement for memcpy that
> would just truncate or panic, 
> 

Oh, I didn't mean to imply it should truncate or panic or such - but if
it returns a pointer it can still be an ERR_PTR() or NULL instead of
having this separate indication, which even often confuses static type
checkers since they don't always see the "errno == 0 <=> ptr != NULL"
relation.

So not saying you shouldn't have any error return - clearly you need
that, just saying that I'm not sure that having the two separated is
great.


> Requiring instance to be NULL is debatable, but I feel pretty strongly
> about it because it does handle a class of mistakes (resource leaks),
> and it's not much of a burden to require a known-good starting state.

Yeah, dunno, I guess I'm slightly more on the side of not requiring it,
since we don't do the same for kmalloc() etc. and probably really
wouldn't want to add kmalloc_s() that does it ;-)

I mean, you _could_ go there:

int kmalloc_s(void **ptr, size_t size, gfp_t gfp)
{
  void *ret;

  if (*ptr)
    return -EINVAL;

  ret = kmalloc(size, gfp);
  if (!ret)
    return -ENOMEM;
  *ptr = ret;
  return 0;  
}

right? But we don't really do that, and I'm not sure it'd be a win if
done over the whole code base.

So I'm not really sure why this aspect here should need to be different,
except of course that you already need the input argument for the magic.

But we could still have (this prototype is theoretical, of course, it
cannot be implemented in C):

void *mem_to_flex_dup(void *ptr, const void *data, size_t elements,
                      gfp_t gfp);


which isn't really that much better though.

And btw, while I was writing it down I was looking to see if it should
be "size_t elements" or "size_t len" (like memcpy), it took me some time
to figure out, and I was looking at the examples:

 1) most of them actually use __u8 or some variant thereof, so you
    could probably add an even simpler macro like
       BOUNDED_FLEX_DATA(int, bytes, data)
    which has the u8 type internally.

 2) Unless I'm confusing myself, you got the firewire change wrong,
    because __mem_to_flex_dup takes the "elements_count", but the
    memcpy() there wasn't multiplied by the sizeof(element)? Or maybe
    the fact that it was declared as __u32 header[0] is wrong, and it
    should be __u8, but it's all very confusing, and I'm really not
    sure about this at all.



One "perhaps you'll laugh me out of the room" suggestion might be to
actually be able to initialize the whole thing too?


mydata = flex_struct_alloc(mydata, GFP_KERNEL,
                           variable_data, variable_len,
                           .member = 1,
                           .another = 2);

(the ordering can't really be otherwise since you have to use
__VA_ARGS__).

That might reduce some more code too, though I guess it's quite some
additional magic ... :)


> > but still, honestly, I don't like it. As APIs go, it feels a bit
> > cumbersome and awkward to use, and you really need everyone to use this,
> > and not say "uh what, I'll memcpy() instead".
> 
> Sure, and I have tried to get it down as small as possible. The earlier
> "just put all the member names in every call" version was horrid. :P

:-D

> I
> realize it's more work to check errno, but the memcpy() API we've all
> been trained to use is just plain dangerous. I don't think it's
> unreasonable to ask people to retrain themselves to avoid it. All that
> said, yes, I want it to be as friendly as possible.
> 
> > Maybe there should also be a realloc() version of it?
> 
> Sure! Seems reasonable. I'd like to see the code pattern for this
> though. Do you have any examples?

I was going to point to struct cfg80211_bss_ies, but I realize now
they're RCU-managed, so we never resize them anyway ... So maybe it's
less common than I thought it might be.

I suppose you know better since you converted a lot of stuff already :-)

johannes
Keith Packard May 5, 2022, 3:16 p.m. UTC | #5
Johannes Berg <johannes@sipsolutions.net> writes:

> Yeah, dunno, I guess I'm slightly more on the side of not requiring it,
> since we don't do the same for kmalloc() etc. and probably really
> wouldn't want to add kmalloc_s() that does it ;-)

I suspect the number of bugs this catches will be small, but they'll be
in places where the flow of control is complicated. What we want is to
know that there's no "real" value already present. I'd love it if we
could make the macro declare a new name (yeah, I know, mixing
declarations and code).

Of course, we could also end up with people writing a wrapping macro
that sets the variable to NULL before invoking the underlying macro...
Kees Cook May 5, 2022, 7:27 p.m. UTC | #6
On Thu, May 05, 2022 at 03:16:19PM +0200, Johannes Berg wrote:
> On Wed, 2022-05-04 at 08:38 -0700, Kees Cook wrote:
> > 
> > It seemed like requiring a structure be rearranged to take advantage of
> > the "automatic layout introspection" wasn't very friendly. On the other
> > hand, looking at the examples, most of them are already neighboring
> > members. Hmmm.
> 
> A lot of them are, and many could be, though not all.

Yeah, I did a pass through them for the coming v2. Only a few have the
struct order as part of an apparent hardware interface.

> > And DECLARE_FLEX_ARRAY_ELEMENTS could actually be expanded to include
> > the count_name too, so both methods could be "forward portable" to a
> > future where C grew the syntax for bounded flex arrays.
> 
> I guess I don't see that happening :)

Well ... it's on my roadmap. ;) I want it for -fsanitize=array-bounds so
that dynamic array indexing can be checked too. (Right now we can do
constant-sized array index bounds checking at runtime, but the much
harder to find problems tend to come from flex arrays.)

> > Requiring instance to be NULL is debatable, but I feel pretty strongly
> > about it because it does handle a class of mistakes (resource leaks),
> > and it's not much of a burden to require a known-good starting state.
> 
> Yeah, dunno, I guess I'm slightly more on the side of not requiring it,
> since we don't do the same for kmalloc() etc. and probably really
> wouldn't want to add kmalloc_s() that does it ;-)

Well, I dislike all the *alloc APIs. :P

> I mean, you _could_ go there:
> 
> int kmalloc_s(void **ptr, size_t size, gfp_t gfp)

Oh, and I really do (though as a macro, not a "real" function), since
having type introspection would be _extremely_ useful. Though maybe it
needs to be through some kind of type-of-lvalue thing...

https://github.com/KSPP/linux/issues/189
https://github.com/KSPP/linux/issues/87

> So I'm not really sure why this aspect here should need to be different,
> except of course that you already need the input argument for the magic.

Right, and trying to move the kernel code closer to a form where the
compiler can take more of the burden of handling code safety.

> And btw, while I was writing it down I was looking to see if it should
> be "size_t elements" or "size_t len" (like memcpy), it took me some time
> to figure out, and I was looking at the examples:
> 
>  1) most of them actually use __u8 or some variant thereof, so you
>     could probably add an even simpler macro like
>        BOUNDED_FLEX_DATA(int, bytes, data)
>     which has the u8 type internally.

I didn't want these helpers to be "opinionated" about their types (just
their API), so while it's true u8 is usually "good enough", I don't
think it's common enough to make a special case for.

>  2) Unless I'm confusing myself, you got the firewire change wrong,
>     because __mem_to_flex_dup takes the "elements_count", but the
>     memcpy() there wasn't multiplied by the sizeof(element)? Or maybe
>     the fact that it was declared as __u32 header[0] is wrong, and it
>     should be __u8, but it's all very confusing, and I'm really not
>     sure about this at all.

Yes indeed; thanks for catching that. In fact, it's not a strict flex
array struct, since, as you say, it's measuring bytes, not elements.
Yeah, I'll see if that needs to be adjusted/dropped, etc.

> One "perhaps you'll laugh me out of the room" suggestion might be to
> actually be able to initialize the whole thing too?
> 
> mydata = flex_struct_alloc(mydata, GFP_KERNEL,
>                            variable_data, variable_len,
>                            .member = 1,
>                            .another = 2);
> 
> (the ordering can't really be otherwise since you have to use
> __VA_ARGS__).

Oooh, that's a cool idea for the API. Hmmmm.

> That might reduce some more code too, though I guess it's quite some
> additional magic ... :)

Yay preprocessor magic!

> I was going to point to struct cfg80211_bss_ies, but I realize now
> they're RCU-managed, so we never resize them anyway ... So maybe it's
> less common than I thought it might be.
> 
> I suppose you know better since you converted a lot of stuff already :-)

Well, I've seen a lot of fragile code (usually in the form of
exploitable flaws around flex arrays) and they do mostly look the same.
Not everything fits perfectly into the forms this API tries to address,
but my goal is to get it fitting well enough, and the weird stuff can be
more carefully examined -- they're easier to find and audit if all the
others are nicely wrapped up in some fancy flex*() API.

Thanks for your thoughts on all of this! I'll continue to work on a v2...

-Kees
Kees Cook May 5, 2022, 7:32 p.m. UTC | #7
On Thu, May 05, 2022 at 08:16:11AM -0700, Keith Packard wrote:
> Johannes Berg <johannes@sipsolutions.net> writes:
> 
> > Yeah, dunno, I guess I'm slightly more on the side of not requiring it,
> > since we don't do the same for kmalloc() etc. and probably really
> > wouldn't want to add kmalloc_s() that does it ;-)
> 
> I suspect the number of bugs this catches will be small, but they'll be
> in places where the flow of control is complicated. What we want is to
> know that there's no "real" value already present. I'd love it if we
> could make the macro declare a new name (yeah, I know, mixing
> declarations and code).

I don't think I can do a declaration and an expression statement at the
same time with different scopes, but that would be kind of cool. We did
just move to c11 to gain the in-loop iterator declarations...

> Of course, we could also end up with people writing a wrapping macro
> that sets the variable to NULL before invoking the underlying macro...

I hope it won't come to that! :)
Keith Packard May 5, 2022, 8:08 p.m. UTC | #8
Kees Cook <keescook@chromium.org> writes:

> I don't think I can do a declaration and an expression statement at the
> same time with different scopes, but that would be kind of cool. We did
> just move to c11 to gain the in-loop iterator declarations...

Yeah, you'd end up creating a statement-level macro, and I think that
would have poor syntax:

        mem_to_flex_dup(struct something *instance, rc, byte_array,
                        count, GFP_KERNEL);
        if (rc)
           return rc;

I bet you've already considered the simpler form:

        struct something *instance = mem_to_flex_dup(byte_array, count, GFP_KERNEL);
        if (IS_ERR(instance))
            return PTR_ERR(instance);

This doesn't allow you to require a new name, so you effectively lose
the check you're trying to insist upon.

Some way to ask the compiler 'is this reference dead?' would be nice --
it knows if a valid pointer was passed to free, or if a variable has not
been initialized, after all; we just need that exposed at the source
level.
Johannes Berg May 5, 2022, 8:12 p.m. UTC | #9
On Thu, 2022-05-05 at 13:08 -0700, Keith Packard wrote:


> I bet you've already considered the simpler form:
> 
>         struct something *instance = mem_to_flex_dup(byte_array, count, GFP_KERNEL);
>         if (IS_ERR(instance))
>             return PTR_ERR(instance);
> 

Sadly, this doesn't work in any way because mem_to_flex_dup() needs to
know at least the type, hence passing 'instance', which is simpler than
passing 'struct something'.

johannes
David Laight May 6, 2022, 11:15 a.m. UTC | #10
From: Johannes Berg
> Sent: 05 May 2022 21:13
> On Thu, 2022-05-05 at 13:08 -0700, Keith Packard wrote:
> 
> 
> > I bet you've already considered the simpler form:
> >
> >         struct something *instance = mem_to_flex_dup(byte_array, count, GFP_KERNEL);
> >         if (IS_ERR(instance))
> >             return PTR_ERR(instance);
> >
> 
> Sadly, this doesn't work in any way because mem_to_flex_dup() needs to
> know at least the type, hence passing 'instance', which is simpler than
> passing 'struct something'.

You can use:
         struct something *instance;
         mem_to_flex_dup(instance, byte_array, count, GFP_KERNEL);
         if (IS_ERR(instance))
             return PTR_ERR(instance);
and have mem_to_flex_dup() (which must be a #define) update 'instance'.
(You can require &instance - and just precede all the uses with
an extra '*' to make it more obvious the variable is updated.
But there is little point requiring it be NULL.)

If you really want to define the variable mid-block you can use:
         mem_to_flex_dup(struct something *, instance, byte_array, count, GFP_KERNEL);

but I really hate having declarations anywhere other than the top of
a function because it makes them hard for the 'mk1 eyeball' to spot.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
diff mbox series

Patch

diff --git a/include/linux/flex_array.h b/include/linux/flex_array.h
new file mode 100644
index 000000000000..b2cf219f7b56
--- /dev/null
+++ b/include/linux/flex_array.h
@@ -0,0 +1,637 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _LINUX_FLEX_ARRAY_H_
+#define _LINUX_FLEX_ARRAY_H_
+
+#include <linux/string.h>
+/*
+ * A "flexible array structure" is a struct which ends with a flexible
+ * array _and_ contains a member that represents how many array elements
+ * are present in the flexible array structure:
+ *
+ * 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.
+ * };
+ *
+ * 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.
+ *
+ * An "encapsulating 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 encapsulating_example {
+ *	...		// other members
+ *	struct flex_array_struct_example fas;
+ * };
+ *
+ * For bounds checking operations on a flexible array structure, member
+ * aliases must be created so the helpers can always locate the associated
+ * members. Marking up the examples above would look like this:
+ *
+ * struct flex_array_struct_example {
+ *	...		// arbitrary members
+ *	// count of elements stored in "parts" below.
+ *	DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(u16, part_count);
+ *	..		// arbitrary members
+ *	// flexible array with elements of type u32.
+ *	DECLARE_FLEX_ARRAY_ELEMENTS(u32, parts);
+ * };
+ *
+ * The above creates the aliases for part_count as __flex_array_elements_count
+ * and parts as __flex_array_elements.
+ *
+ * For encapsulated flexible array structs, there are alternative helpers
+ * below where the flexible array struct member name can be explicitly
+ * included as an argument. (See the @dot_fas_member arguments below.)
+ *
+ *
+ * Examples:
+ *
+ * Using mem_to_flex():
+ *
+ *        struct single {
+ *                u32 flags;
+ *                u32 count;
+ *                u8 data[];
+ *        };
+ *        struct single *ptr_single;
+ *
+ *        struct encap {
+ *                u16 info;
+ *                struct single single;
+ *        };
+ *        struct encap *ptr_encap;
+ *
+ *        struct blob {
+ *                u32 flags;
+ *                u8 data[];
+ *        };
+ *
+ *        struct split {
+ *                u32 count;
+ *                struct blob blob;
+ *        };
+ *        struct split *ptr_split;
+ *
+ *        mem_to_flex(ptr_one, src, count);
+ *        __mem_to_flex(ptr_encap, single.data, single.count, src, count);
+ *        __mem_to_flex(ptr_split, count, blob.data, src, count);
+ *
+ */
+
+/* These are wrappers around the UAPI macros. */
+#define DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(TYPE, NAME)			\
+	__DECLARE_FLEX_ARRAY_ELEMENTS_COUNT(TYPE, NAME)
+
+#define DECLARE_FLEX_ARRAY_ELEMENTS(TYPE, NAME)				\
+	__DECLARE_FLEX_ARRAY_ELEMENTS(TYPE, NAME)
+
+/* All the helpers return negative on failure, as must be checked. */
+static inline int __must_check __must_check_errno(int err)
+{
+	return err;
+}
+
+/**
+ * __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 with count conversion
+ *
+ * @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) __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, &__fc_dst_bytes) ||		\
+		    fas_bytes(__fc_src, &__fc_src_bytes) ||		\
+		    __fc_dst_bytes < __fc_src_bytes) {			\
+			/* do we need to wipe dst here? */		\
+			__fc_err = -E2BIG;				\
+			break;						\
+		}							\
+		__builtin_memcpy(__fc_dst, __fc_src, __fc_src_bytes);	\
+		/* __flex_array_elements_count is included in memcpy */	\
+		/* Wipe any now-unused trailing elements in @dst: */	\
+		__builtin_memset((u8 *)__fc_dst + __fc_src_bytes, 0,	\
+				 __fc_dst_bytes - __fc_src_bytes);	\
+		__fc_err = 0;						\
+	} while (0);							\
+	__fc_err;							\
+}))
+
+/** __flex_dup - Allocate and copy an arbitrarily encapsulated flexible
+ *		 array struct
+ *
+ * @alloc: Pointer to Pointer to hold to-be-allocated (optionally
+ *	   encapsulating) flexible array struct.
+ * @dot_fas_member: For encapsulating flexible arrays, the name of the
+ *		    flexible array struct member preceded with a literal
+ *		    dot (e.g. .foo.bar.flex_array_struct_name). For a
+ *		    regular flexible array struct, this macro arument is
+ *		    empty.
+ * @src: Pointer to source flexible array struct.
+ * @gfp: GFP allocation flags
+ *
+ * This copies the contents of one flexible array struct into another.
+ * The (**@alloc)@dot_fas_member and @src arguments must resolve to the
+ * same type. Everything prior to @dot_fas_member in *@alloc will be
+ * initialized to zero.
+ *
+ * Failure modes:
+ * - @alloc is NULL.
+ * - *@alloc is not NULL (something was already allocated).
+ * - Required allocation size is larger than size_t can hold.
+ * - No available memory to allocate @alloc.
+ *
+ * Returns: 0 on success, -ve on failure.
+ */
+#define __flex_dup(alloc, dot_fas_member, src, gfp)			\
+__must_check_errno(({							\
+	int __fd_err = -EINVAL;						\
+	typeof(*(src)) *__fd_src = (src);				\
+	typeof(**(alloc)) *__fd_alloc;					\
+	typeof((*__fd_alloc)dot_fas_member) *__fd_dst;			\
+	size_t __fd_alloc_bytes, __fd_copy_bytes;			\
+									\
+	BUILD_BUG_ON(!__same_type(*(__fd_dst), *(__fd_src)));		\
+									\
+	do {								\
+		if ((uintptr_t)(alloc) < 1 || *(alloc)) {		\
+			__fd_err = -EINVAL;				\
+			break;						\
+		}							\
+		if (fas_bytes(__fd_src, &__fd_copy_bytes) ||		\
+		    check_add_overflow(__fd_copy_bytes,			\
+				       sizeof(*__fd_alloc) -		\
+					sizeof(*__fd_dst),		\
+				       &__fd_alloc_bytes)) {		\
+			__fd_err = -E2BIG;				\
+			break;						\
+		}							\
+		__fd_alloc = kmalloc(__fd_alloc_bytes, gfp);		\
+		if (!__fd_alloc) {					\
+			__fd_err = -ENOMEM;				\
+			break;						\
+		}							\
+		__fd_dst = &((*__fd_alloc)dot_fas_member);		\
+		/* Optimize away any unneeded memset. */		\
+		if (sizeof(*__fd_alloc) != sizeof(*__fd_dst))		\
+			__builtin_memset(__fd_alloc, 0,			\
+					 __fd_alloc_bytes -		\
+						__fd_copy_bytes);	\
+		__builtin_memcpy(__fd_dst, src, __fd_copy_bytes);	\
+		/* __flex_array_elements_count is included in memcpy */	\
+		*(alloc) = __fd_alloc;					\
+		__fd_err = 0;						\
+	} while (0);							\
+	__fd_err;							\
+}))
+
+/** flex_dup - Allocate and copy a flexible array struct
+ *
+ * @alloc: Pointer to Pointer to hold to-be-allocated flexible array struct.
+ * @src: Pointer to source flexible array struct.
+ * @gfp: GFP allocation flags
+ *
+ * This copies the contents of one flexible array struct into another.
+ * The *@alloc and @src arguments must resolve to the same type.
+ *
+ * Failure modes:
+ * - @alloc is NULL.
+ * - *@alloc is not NULL (something was already allocated).
+ * - Required allocation size is larger than size_t can hold.
+ * - No available memory to allocate @alloc.
+ *
+ * Returns: 0 on success, -ve on failure.
+ */
+#define flex_dup(alloc, src, gfp)					\
+	__flex_dup(alloc, /* alloc itself */, src, gfp)
+
+/** __mem_to_flex - Copy from memory buffer into a flexible array structure's
+ *		    flexible array elements.
+ *
+ * @ptr: Pointer to already allocated flexible array struct.
+ * @flex_member: Member name of the flexible array elements.
+ * @count_member: Member name of the flexible array elements count.
+ * @src: Source memory pointer.
+ * @elements_count: Number of @ptr's flexible array elements to copy from
+ *		    @src into @ptr.
+ *
+ * Copies @elements_count-many elements from memory buffer at @src into
+ * @ptr->@flex_member, wipes any remaining elements, and updates
+ * @ptr->@count_member.
+ *
+ * This is essentially a simple deserializer.
+ *
+ * TODO: It would be nice to automatically discover the max bounds of @src
+ *	 besides @elements_count. There is currently no universal way to ask
+ *	 "what is the size of a given pointer's allocation?" So for
+ *	 now just use __builtin_object_size(@src, 1) to validate known
+ *	 compile-time too-large conditions. Perhaps in the future if
+ *	 __mtf_copy_bytes above is > PAGE_SIZE, perform a dynamic lookup
+ *	 using something similar to __check_heap_object().
+ *
+ * Failure conditions:
+ * - The value of @elements_count cannot fit in the @ptr's @count_member
+ *   type (e.g. 260 in a u8).
+ * - @ptr's @count_member value is smaller than @elements_count (e.g. not
+ *   enough space was previously allocated).
+ * - @elements_count yields a byte count greater than:
+ *   - INT_MAX (as a simple "too big" sanity check)
+ *   - the compile-time size of @src (when it can be determined)
+ *
+ * Returns: 0 on success, -ve on error.
+ */
+#define __mem_to_flex(ptr, flex_member, count_member, src,		\
+		      elements_count)					\
+__must_check_errno(({							\
+	int __mtf_err = -EINVAL;					\
+	typeof(*(ptr)) *__mtf_ptr = (ptr);				\
+	typeof(elements_count) __mtf_src_count = (elements_count);	\
+	size_t __mtf_copy_bytes, __mtf_dst_bytes;			\
+	u8 *__mtf_dst = (u8 *)__mtf_ptr->flex_member;			\
+									\
+	do {								\
+		if (is_negative(__mtf_src_count) ||			\
+		    __fas_elements_bytes(__mtf_ptr, flex_member,	\
+					 count_member,			\
+					 __mtf_src_count,		\
+					 &__mtf_copy_bytes) ||		\
+		    __mtf_copy_bytes > INT_MAX ||			\
+		    __mtf_copy_bytes > __builtin_object_size(src, 1) ||	\
+		    __fas_elements_bytes(__mtf_ptr, flex_member,	\
+					 count_member,			\
+					 __mtf_ptr->count_member,	\
+					 &__mtf_dst_bytes) ||		\
+		    __mtf_dst_bytes < __mtf_copy_bytes) {		\
+			__mtf_err = -E2BIG;				\
+			break;						\
+		}							\
+		__builtin_memcpy(__mtf_dst, src, __mtf_copy_bytes);	\
+		/* Wipe any now-unused trailing elements in @dst: */	\
+		__builtin_memset(__mtf_dst + __mtf_dst_bytes, 0,	\
+				 __mtf_dst_bytes - __mtf_copy_bytes);	\
+		/* Make sure in-struct count of elements is updated: */	\
+		__mtf_ptr->count_member = __mtf_src_count;		\
+		__mtf_err = 0;						\
+	} while (0);							\
+	__mtf_err;							\
+}))
+
+#define mem_to_flex(ptr, src, elements_count)				\
+	__mem_to_flex(ptr, __flex_array_elements,			\
+		      __flex_array_elements_count, src, elements_count)
+
+/** __mem_to_flex_dup - Allocate a flexible array structure and copy into
+ *			its flexible array elements from a memory buffer.
+ *
+ * @alloc: Pointer to pointer to hold allocation for flexible array struct.
+ * @dot_fas_member: For encapsulating flexible array structs, the name of
+ *		    the flexible array struct member preceded with a
+ *		    literal dot (e.g. .foo.bar.flex_array_struct_name).
+ *		    For a regular flexible array struct, this macro arument
+ *		    is empty.
+ * @src: Source memory buffer pointer.
+ * @elements_count: Number of @alloc's flexible array elements to copy from
+ *		    @src into @ptr.
+ * @gfp: GFP allocation flags
+ *
+ * This behaves like mem_to_flex(), but allocates the needed space for
+ * a new flexible array struct and its trailing elements.
+ *
+ * This is essentially a simple allocating deserializer.
+ *
+ * TODO: It would be nice to automatically discover the max bounds of @src
+ *	 besides @elements_count. There is currently no universal way to ask
+ *	 "what is the size of a given pointer's allocation?" So for now just
+ *	 use __builtin_object_size(@src, 1) to validate known compile-time
+ *	 too-large conditions. Perhaps in the future if __mtfd_copy_bytes
+ *	 above is > PAGE_SIZE, perform a dynamic lookup using something
+ *	 similar to __check_heap_object().
+ *
+ * Failure conditions:
+ * - @alloc is NULL.
+ * - *@alloc is not NULL (something was already allocated).
+ * - The value of @elements_count cannot fit in the @alloc's
+ *   __flex_array_elements_count member type (e.g. 260 in u8).
+ * - @elements_count yields a byte count greater than:
+ *   - INT_MAX (as a simple "too big" sanity check)
+ *   - the compile-time size of @src (when it can be determined)
+ * - @alloc could not be allocated.
+ *
+ * Returns: 0 on success, -ve on error.
+ */
+#define __mem_to_flex_dup(alloc, dot_fas_member, src, elements_count,	\
+			  gfp)						\
+__must_check_errno(({							\
+	int __mtfd_err = -EINVAL;					\
+	typeof(elements_count) __mtfd_src_count = (elements_count);	\
+	typeof(**(alloc)) *__mtfd_alloc;				\
+	typeof((*__mtfd_alloc)dot_fas_member) *__mtfd_fas;		\
+	u8 *__mtfd_dst;							\
+	size_t __mtfd_alloc_bytes, __mtfd_copy_bytes;			\
+									\
+	do {								\
+		if ((uintptr_t)(alloc) < 1 || *(alloc)) {		\
+			__mtfd_err = -EINVAL;				\
+			break;						\
+		}							\
+		if (is_negative(__mtfd_src_count) ||			\
+		    __fas_elements_bytes(__mtfd_fas,			\
+					 __flex_array_elements,		\
+					 __flex_array_elements_count,	\
+					 __mtfd_src_count,		\
+					 &__mtfd_copy_bytes) ||		\
+		    __mtfd_copy_bytes > INT_MAX ||			\
+		    __mtfd_copy_bytes > __builtin_object_size(src, 1) ||\
+		    check_add_overflow(sizeof(*__mtfd_alloc),		\
+				       __mtfd_copy_bytes,		\
+				       &__mtfd_alloc_bytes)) {		\
+			__mtfd_err = -E2BIG;				\
+			break;						\
+		}							\
+		__mtfd_alloc = kmalloc(__mtfd_alloc_bytes, gfp);	\
+		if (!__mtfd_alloc) {					\
+			__mtfd_err = -ENOMEM;				\
+			break;						\
+		}							\
+		__mtfd_fas = &((*__mtfd_alloc)dot_fas_member);		\
+		__mtfd_dst = (u8 *)__mtfd_fas->__flex_array_elements;	\
+		__builtin_memset(__mtfd_alloc, 0, __mtfd_alloc_bytes -	\
+						  __mtfd_copy_bytes);	\
+		__builtin_memcpy(__mtfd_dst, src, __mtfd_copy_bytes);	\
+		/* Make sure in-struct count of elements is updated: */	\
+		__mtfd_fas->__flex_array_elements_count =		\
+						    __mtfd_src_count;	\
+		*(alloc) = __mtfd_alloc;				\
+		__mtfd_err = 0;						\
+	} while (0);							\
+	__mtfd_err;							\
+}))
+
+/** mem_to_flex_dup - Allocate a flexible array structure and copy
+ *			into it from a memory buffer.
+ *
+ * @alloc: Pointer to pointer to hold allocation for flexible array struct.
+ * @src: Source memory pointer.
+ * @elements_count: Number of @alloc's flexible array elements to copy from
+ *		   @src into @alloc.
+ * @gfp: GFP allocation flags
+ *
+ * This behaves like mem_to_flex(), but allocates the needed space for
+ * a new flexible array struct and its trailing elements.
+ *
+ * This is essentially a simple allocating deserializer.
+ *
+ * TODO: It would be nice to automatically discover the max bounds of @src
+ *	 besides @elements_count. There is currently no universal way to ask
+ *	 "what is the size of a given pointer's allocation?" So for
+ *	 now just use __builtin_object_size(@src, 1) to validate known
+ *	 compile-time too-large conditions. Perhaps in the future if
+ *	 __mtf_copy_bytes above is > PAGE_SIZE, perform a dynamic lookup
+ *	 using something similar to __check_heap_object().
+ *
+ * Failure conditions:
+ * - @alloc is NULL.
+ * - *@alloc is not NULL (something was already allocated).
+ * - The value of @elements_count cannot fit in the @alloc's
+ *   __flex_array_elements_count member type (e.g. 260 in u8).
+ * - @elements_count yields a byte count greater than:
+ *   - INT_MAX (as a simple "too big" sanity check)
+ *   - the compile-time size of @src (when it can be determined)
+ * - @alloc could not be allocated.
+ *
+ * Returns: 0 on success, -ve on error.
+ */
+#define mem_to_flex_dup(alloc, src, elements_count, gfp)		\
+	__mem_to_flex_dup(alloc, /* alloc itself */, src, elements_count, gfp)
+
+/** flex_to_mem - Copy all flexible array structure elements into memory
+ *		  buffer.
+ *
+ * @dst: Destination buffer pointer.
+ * @bytes_available: How many bytes are available in @dst.
+ * @ptr: Pointer to allocated flexible array struct.
+ * @bytes_written: Pointer to variable to store how many bytes were written
+ *		   (may be NULL).
+ *
+ * Copies all of @ptr's flexible array elements into @dst.
+ *
+ * This is essentially a simple serializer.
+ *
+ * Failure conditions:
+ * - @bytes_available in @dst is any of:
+ *   - negative.
+ *   - larger than INT_MAX.
+ *   - not large enough to hold the resulting copy.
+ * - @bytes_written's type cannot hold the size of the copy (e.g. 260 in u8).
+ *
+ * Return: 0 on success, -ve on failure.
+ *
+ */
+#define flex_to_mem(dst, bytes_available, ptr, bytes_written)		\
+__must_check_errno(({							\
+	int __ftm_err = -EINVAL;					\
+	typeof(*(ptr)) *__ftm_ptr = (ptr);				\
+	u8 *__ftm_src = (u8 *)__ftm_ptr->__flex_array_elements;		\
+	typeof(*(bytes_written)) *__ftm_written = (bytes_written);	\
+	size_t __ftm_written_max = type_max(typeof(*__ftm_written));	\
+	typeof(bytes_available) __ftm_dst_bytes = (bytes_available);	\
+	size_t __ftm_copy_bytes;					\
+									\
+	do {								\
+		if (is_negative(__ftm_dst_bytes) ||			\
+		    __ftm_dst_bytes > INT_MAX ||			\
+		    fas_elements_bytes(__ftm_ptr, &__ftm_copy_bytes) ||	\
+		    __ftm_dst_bytes < __ftm_copy_bytes ||		\
+		    (!__same_type(typeof(bytes_written), NULL) &&	\
+		     __ftm_copy_bytes > __ftm_written_max)) {		\
+			__ftm_err = -E2BIG;				\
+			break;						\
+		}							\
+		__builtin_memcpy(dst, __ftm_src, __ftm_copy_bytes);	\
+		if (__ftm_written)					\
+			*__ftm_written = __ftm_copy_bytes;		\
+		__ftm_err = 0;						\
+	} while (0);							\
+	__ftm_err;							\
+}))
+
+/** flex_to_mem_dup - Copy entire flexible array structure into newly
+ *		      allocated memory buffer.
+ *
+ * @alloc: Pointer to pointer to newly allocated memory region to hold contents
+ *	   of the copy.
+ * @alloc_size: Pointer to variable to hold the size of the allocated memory.
+ * @ptr: Pointer to allocated flexible array struct.
+ * @gfp: GFP allocation flags
+ *
+ * Allocates @alloc and copies all of @ptr's flexible array elements.
+ *
+ * This is essentially a simple allocating serializer.
+ *
+ * Failure conditions:
+ * - @alloc is NULL.
+ * - *@alloc is not NULL (something was already allocated).
+ * - @alloc_size is NULL.
+ * - @alloc_size's type cannot hold the size of the copy (e.g. 260 in u8).
+ * - @alloc could not be allocated.
+ *
+ * Return: 0 on success, -ve on failure.
+ */
+#define flex_to_mem_dup(alloc, alloc_size, ptr, gfp)			\
+__must_check_errno(({							\
+	int __ftmd_err = -EINVAL;					\
+	typeof(**(alloc)) *__ftmd_alloc;				\
+	typeof(*(alloc_size)) *__ftmd_alloc_size = (alloc_size);	\
+	typeof(*(ptr)) *__ftmd_ptr = (ptr);				\
+	u8 *__ftmd_src = (u8 *)__ftmd_ptr->__flex_array_elements;	\
+	size_t __ftmd_alloc_max = type_max(typeof(*__ftmd_alloc_size));	\
+	size_t __ftmd_copy_bytes;					\
+									\
+	do {								\
+		if ((uintptr_t)(alloc) < 1 || *(alloc) ||		\
+		    (uintptr_t)(alloc_size) < 1) {			\
+			__ftmd_err = -EINVAL;				\
+			break;						\
+		}							\
+		if (fas_elements_bytes(__ftmd_ptr,			\
+				       &__ftmd_copy_bytes) ||		\
+		    __ftmd_copy_bytes > __ftmd_alloc_max) {		\
+			__ftmd_err = -E2BIG;				\
+			break;						\
+		}							\
+		__ftmd_alloc = kmemdup(__ftmd_src, __ftmd_copy_bytes,	\
+				       gfp);				\
+		if (!__ftmd_alloc) {					\
+			__ftmd_err = -ENOMEM;				\
+			break;						\
+		}							\
+		*__ftmd_alloc_size = __ftmd_copy_bytes;			\
+		*(alloc) = __ftmd_alloc;				\
+		__ftmd_err = 0;						\
+	} while (0);							\
+	__ftmd_err;							\
+}))
+
+#endif /* _LINUX_FLEX_ARRAY_H_ */
diff --git a/include/linux/string.h b/include/linux/string.h
index b6572aeca2f5..c01b76f73e99 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -252,6 +252,7 @@  static inline const char *kbasename(const char *path)
 #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE)
 #include <linux/fortify-string.h>
 #endif
+#include <linux/flex_array.h>
 
 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..04870274f33b 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_FLEX_ARRAY_ELEMENTS_COUNT(TYPE, NAME)			\
+	union {								\
+		TYPE __flex_array_elements_count;			\
+		TYPE NAME;						\
+	}
+
+#define __DECLARE_FLEX_ARRAY_ELEMENTS(TYPE, NAME)			\
+	union {								\
+		__DECLARE_FLEX_ARRAY(TYPE, __flex_array_elements);	\
+		__DECLARE_FLEX_ARRAY(TYPE, NAME);			\
+	}
+
 #endif