Skip to content

Commit bb5e62f

Browse files
gal-pressmankuba-moo
authored andcommitted
net: Add options as a flexible array to struct ip_tunnel_info
Remove the hidden assumption that options are allocated at the end of the struct, and teach the compiler about them using a flexible array. With this, we can revert the unsafe_memcpy() call we have in tun_dst_unclone() [1], and resolve the false field-spanning write warning caused by the memcpy() in ip_tunnel_info_opts_set(). The layout of struct ip_tunnel_info remains the same with this patch. Before this patch, there was an implicit padding at the end of the struct, options would be written at 'info + 1' which is after the padding. This will remain the same as this patch explicitly aligns 'options'. The alignment is needed as the options are later casted to different structs, and might result in unaligned memory access. Pahole output before this patch: struct ip_tunnel_info { struct ip_tunnel_key key; /* 0 64 */ /* XXX last struct has 1 byte of padding */ /* --- cacheline 1 boundary (64 bytes) --- */ struct ip_tunnel_encap encap; /* 64 8 */ struct dst_cache dst_cache; /* 72 16 */ u8 options_len; /* 88 1 */ u8 mode; /* 89 1 */ /* size: 96, cachelines: 2, members: 5 */ /* padding: 6 */ /* paddings: 1, sum paddings: 1 */ /* last cacheline: 32 bytes */ }; Pahole output after this patch: struct ip_tunnel_info { struct ip_tunnel_key key; /* 0 64 */ /* XXX last struct has 1 byte of padding */ /* --- cacheline 1 boundary (64 bytes) --- */ struct ip_tunnel_encap encap; /* 64 8 */ struct dst_cache dst_cache; /* 72 16 */ u8 options_len; /* 88 1 */ u8 mode; /* 89 1 */ /* XXX 6 bytes hole, try to pack */ u8 options[] __attribute__((__aligned__(16))); /* 96 0 */ /* size: 96, cachelines: 2, members: 6 */ /* sum members: 90, holes: 1, sum holes: 6 */ /* paddings: 1, sum paddings: 1 */ /* forced alignments: 1, forced holes: 1, sum forced holes: 6 */ /* last cacheline: 32 bytes */ } __attribute__((__aligned__(16))); [1] Commit 13cfd6a ("net: Silence false field-spanning write warning in metadata_dst memcpy") Link: https://lore.kernel.org/all/[email protected]/ Suggested-by: Kees Cook <[email protected]> Reviewed-by: Cosmin Ratiu <[email protected]> Reviewed-by: Tariq Toukan <[email protected]> Signed-off-by: Gal Pressman <[email protected]> Reviewed-by: Kees Cook <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent ba3fa6e commit bb5e62f

File tree

3 files changed

+9
-9
lines changed

3 files changed

+9
-9
lines changed

include/net/dst_metadata.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,8 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb)
163163
if (!new_md)
164164
return ERR_PTR(-ENOMEM);
165165

166-
unsafe_memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
167-
sizeof(struct ip_tunnel_info) + md_size,
168-
/* metadata_dst_alloc() reserves room (md_size bytes) for
169-
* options right after the ip_tunnel_info struct.
170-
*/);
166+
memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
167+
sizeof(struct ip_tunnel_info) + md_size);
171168
#ifdef CONFIG_DST_CACHE
172169
/* Unclone the dst cache if there is one */
173170
if (new_md->u.tun_info.dst_cache.cache) {

include/net/ip_tunnels.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ struct ip_tunnel_encap {
9595

9696
#define ip_tunnel_info_opts(info) \
9797
_Generic(info, \
98-
const struct ip_tunnel_info * : ((const void *)((info) + 1)),\
99-
struct ip_tunnel_info * : ((void *)((info) + 1))\
98+
const struct ip_tunnel_info * : ((const void *)(info)->options),\
99+
struct ip_tunnel_info * : ((void *)(info)->options)\
100100
)
101101

102102
struct ip_tunnel_info {
@@ -107,6 +107,7 @@ struct ip_tunnel_info {
107107
#endif
108108
u8 options_len;
109109
u8 mode;
110+
u8 options[] __aligned_largest __counted_by(options_len);
110111
};
111112

112113
/* 6rd prefix/relay information */

net/core/dst.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,8 @@ struct metadata_dst *metadata_dst_alloc(u8 optslen, enum metadata_type type,
286286
{
287287
struct metadata_dst *md_dst;
288288

289-
md_dst = kmalloc(sizeof(*md_dst) + optslen, flags);
289+
md_dst = kmalloc(struct_size(md_dst, u.tun_info.options, optslen),
290+
flags);
290291
if (!md_dst)
291292
return NULL;
292293

@@ -314,7 +315,8 @@ metadata_dst_alloc_percpu(u8 optslen, enum metadata_type type, gfp_t flags)
314315
int cpu;
315316
struct metadata_dst __percpu *md_dst;
316317

317-
md_dst = __alloc_percpu_gfp(sizeof(struct metadata_dst) + optslen,
318+
md_dst = __alloc_percpu_gfp(struct_size(md_dst, u.tun_info.options,
319+
optslen),
318320
__alignof__(struct metadata_dst), flags);
319321
if (!md_dst)
320322
return NULL;

0 commit comments

Comments
 (0)