Skip to content

Commit

Permalink
net: Add options as a flexible array to struct ip_tunnel_info
Browse files Browse the repository at this point in the history
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/53D1D353-B8F6-4ADC-8F29-8C48A7C9C6F1@kernel.org/
Suggested-by: Kees Cook <kees@kernel.org>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Signed-off-by: Gal Pressman <gal@nvidia.com>
Reviewed-by: Kees Cook <kees@kernel.org>
Link: https://patch.msgid.link/20250219143256.370277-3-gal@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Gal Pressman authored and Jakub Kicinski committed Feb 20, 2025
1 parent ba3fa6e commit bb5e62f
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 9 deletions.
7 changes: 2 additions & 5 deletions include/net/dst_metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,11 +163,8 @@ static inline struct metadata_dst *tun_dst_unclone(struct sk_buff *skb)
if (!new_md)
return ERR_PTR(-ENOMEM);

unsafe_memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
sizeof(struct ip_tunnel_info) + md_size,
/* metadata_dst_alloc() reserves room (md_size bytes) for
* options right after the ip_tunnel_info struct.
*/);
memcpy(&new_md->u.tun_info, &md_dst->u.tun_info,
sizeof(struct ip_tunnel_info) + md_size);
#ifdef CONFIG_DST_CACHE
/* Unclone the dst cache if there is one */
if (new_md->u.tun_info.dst_cache.cache) {
Expand Down
5 changes: 3 additions & 2 deletions include/net/ip_tunnels.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ struct ip_tunnel_encap {

#define ip_tunnel_info_opts(info) \
_Generic(info, \
const struct ip_tunnel_info * : ((const void *)((info) + 1)),\
struct ip_tunnel_info * : ((void *)((info) + 1))\
const struct ip_tunnel_info * : ((const void *)(info)->options),\
struct ip_tunnel_info * : ((void *)(info)->options)\
)

struct ip_tunnel_info {
Expand All @@ -107,6 +107,7 @@ struct ip_tunnel_info {
#endif
u8 options_len;
u8 mode;
u8 options[] __aligned_largest __counted_by(options_len);
};

/* 6rd prefix/relay information */
Expand Down
6 changes: 4 additions & 2 deletions net/core/dst.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ struct metadata_dst *metadata_dst_alloc(u8 optslen, enum metadata_type type,
{
struct metadata_dst *md_dst;

md_dst = kmalloc(sizeof(*md_dst) + optslen, flags);
md_dst = kmalloc(struct_size(md_dst, u.tun_info.options, optslen),
flags);
if (!md_dst)
return NULL;

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

md_dst = __alloc_percpu_gfp(sizeof(struct metadata_dst) + optslen,
md_dst = __alloc_percpu_gfp(struct_size(md_dst, u.tun_info.options,
optslen),
__alignof__(struct metadata_dst), flags);
if (!md_dst)
return NULL;
Expand Down

0 comments on commit bb5e62f

Please sign in to comment.