stddef: Allow attributes to be used when creating flex arrays
Subsystems (2)
v2 cover stddef: Allow attributes to be used when creating flex arrays
v2:
- don't add a new helper, just add __VA_ARGS__ (Rasmus)
v1: https://lore.kernel.org/all/[email protected]/
Hi,
We're going to have more cases where we need to apply attributes (e.g. __counted_by) to struct members that have been declared with DECLARE_FLEX_ARRAY. Add an optional 3rd argument to allow for this and annotate one such user in linux/in.h.
I kept the acks/reviews since it's effectively the same...
-Kees
Kees Cook (2):
stddef: Allow attributes to be used when creating flex arrays net/ipv4: Annotate imsf_slist_flex with __counted_by(imsf_numsrc)
include/linux/stddef.h | 6 +++--- include/uapi/linux/in.h | 3 ++- include/uapi/linux/stddef.h | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-)
Signature · 1 line
2.34.1
PATCH v2 1/2 stddef: Allow attributes to be used when creating flex arrays
With the coming support for the __counted_by struct member attribute, we will need a way to add such annotations to the places where DECLARE_FLEX_ARRAY() is used. Add an optional 3rd argument that can be used for including attributes in the flexible array definition.
--- include/linux/stddef.h | 6 +++--- include/uapi/linux/stddef.h | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-)
include/linux/stddef.h +3 -3
@@ -82,15 +82,15 @@ enum {/** * DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union -* * @TYPE: The type of each flexible array element * @NAME: The name of the flexible array member +* @...: The list of member attributes to apply (optional) * * In order to have a flexible array member in a union or alone in a * struct, it needs to be wrapped in an anonymous struct with at least 1 * named member, but that member can be empty. */ -#define DECLARE_FLEX_ARRAY (TYPE ,NAME ) \ - __DECLARE_FLEX_ARRAY(TYPE, NAME) +#define DECLARE_FLEX_ARRAY (TYPE ,NAME , ...) \ + __DECLARE_FLEX_ARRAY(TYPE, NAME, __VA_ARGS__)#endif
include/uapi/linux/stddef.h +5 -5
@@ -31,23 +31,23 @@#ifdef __cplusplus/* sizeof(struct{}) is 1 in C++, not 0, can't use C version of the macro. */ -#define __DECLARE_FLEX_ARRAY (T , member) \ - T member[0] +#define __DECLARE_FLEX_ARRAY (TYPE ,NAME , ...) \ + TYPE NAME[0] __VA_ARGS__#else /** * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union -* * @TYPE: The type of each flexible array element * @NAME: The name of the flexible array member +* @...: The list of member attributes to apply (optional) * * In order to have a flexible array member in a union or alone in a * struct, it needs to be wrapped in an anonymous struct with at least 1 * named member, but that member can be empty. */ -#define __DECLARE_FLEX_ARRAY (TYPE ,NAME ) \ +#define __DECLARE_FLEX_ARRAY (TYPE ,NAME , ...) \ struct { \ struct { } __empty_ ## NAME; \ - TYPE NAME[]; \ + TYPE NAME[] __VA_ARGS__; \ }#endif
Signature · 1 line
2.34.1
Hi, Kees
I was looking to apply the __counted_by to the drivers/net/can subtree, and a research on the DECLARE_FLEX_ARRAY brought me to this patch.
I could not find it in any tree (tried Linus's tree and linux-next), so I am not sure what is the status here (sorry if it was upstreamed and if I just missed it).
While at it, and with several months of delays, here is my feedback.
Quoted 23 lines · Kees Cook
With the coming support for the
__counted_bystruct member attribute, we will need a way to add such annotations to the places whereDECLARE_FLEX_ARRAY()is used. Add an optional 3rd argument that can be used for including attributes in the flexible array definition.Cc Rasmus Villemoes <[email protected]>Cc Dan Williams <[email protected]>Cc Keith Packard <[email protected]>Cc Miguel Ojeda <[email protected]>Cc Alexey Dobriyan <[email protected]>Cc Dmitry Antipov <[email protected]>Reviewed-by Gustavo A. R. Silva <[email protected]>Signed-off-by Kees Cook <[email protected]>--- include/linux/stddef.h | 6 +++--- include/uapi/linux/stddef.h | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-)include/linux/stddef.h +0
-0
include/linux/stddef.h enum { · line 85
Quoted 4 lines
include/linux/stddef.h +0
-1@@ -82,15 +82,15 @@ enum {/** * DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union -*
Nitpick: this line removal is not related to the patch and the other documentation blocks in include/linux/stddef.h also have this empty line. For consistency, better to keep.
Quoted 20 lines
include/linux/stddef.h +3
-2* @TYPE: The type of each flexible array element * @NAME: The name of the flexible array member +* @...: The list of member attributes to apply (optional) * * In order to have a flexible array member in a union or alone in a * struct, it needs to be wrapped in an anonymous struct with at least 1 * named member, but that member can be empty. */ -#define DECLARE_FLEX_ARRAY (TYPE ,NAME ) \ - __DECLARE_FLEX_ARRAY(TYPE, NAME) +#define DECLARE_FLEX_ARRAY (TYPE ,NAME , ...) \ + __DECLARE_FLEX_ARRAY(TYPE, NAME, __VA_ARGS__)#endif include/uapi/linux/stddef.h +0
-0@@ -31,23 +31,23 @@
include/uapi/linux/stddef.h line 39
Quoted 10 lines
include/uapi/linux/stddef.h +2
-3#ifdef __cplusplus/* sizeof(struct{}) is 1 in C++, not 0, can't use C version of the macro. */ -#define __DECLARE_FLEX_ARRAY (T , member) \ - T member[0] +#define __DECLARE_FLEX_ARRAY (TYPE ,NAME , ...) \ + TYPE NAME[0] __VA_ARGS__#else /** * __DECLARE_FLEX_ARRAY() - Declare a flexible array usable in a union -*
Same as above: no need to remove.
Quoted 6 lines
include/uapi/linux/stddef.h +1
-0* @TYPE: The type of each flexible array element * @NAME: The name of the flexible array member +* @...: The list of member attributes to apply (optional) * * In order to have a flexible array member in a union or alone in a * struct, it needs to be wrapped in an anonymous struct with at least 1
include/uapi/linux/stddef.h line 52
Quoted 10 lines
include/uapi/linux/stddef.h +2
-2* named member, but that member can be empty. */ -#define __DECLARE_FLEX_ARRAY (TYPE ,NAME ) \ +#define __DECLARE_FLEX_ARRAY (TYPE ,NAME , ...) \ struct { \ struct { } __empty_ ## NAME; \ - TYPE NAME[]; \ + TYPE NAME[] __VA_ARGS__; \ }#endif
How does this work?
If I take this example:
struct foo {
size_t union_size;
union {
struct bar;
DECLARE_FLEX_ARRAY(u8, raw, __counted_by(union_size));
};
};it will expand to:
struct foo {
size_t union_size;
union {
struct bar;
struct {
struct { } __empty_raw;
u8 raw[] __counted_by(union_size);
}; };
};
right?
Looking at clang documentation:
The count field member must be within the same non-anonymous, enclosing struct as the flexible array member.
Ref: https://clang.llvm.org/docs/AttributeReference.html#counted-by
Here, the union_size and the flexible array member are in different structures (struct foo and anonymous structure). It seems to me that the prerequisites are not met. Am I missing something?
Yours sincerely,
Vincent Mailhol
PATCH v2 2/2 net/ipv4: Annotate imsf_slist_flex with __counted_by(imsf_numsrc)
In reply to Kees CookThe size of the imsf_slist_flex member is determined by imsf_numsrc, so annotate it as such.
--- include/uapi/linux/in.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/uapi/linux/in.h +2 -1
@@ -199,7 +199,8 @@ struct ip_msfilter {__u32 imsf_numsrc;union {__be32 imsf_slist[1 ]; - __DECLARE_FLEX_ARRAY(__be32, imsf_slist_flex); + __DECLARE_FLEX_ARRAY(__be32, imsf_slist_flex, + __counted_by(imsf_numsrc )); }; };
Signature · 1 line
2.34.1