We recently found out that extending xsk_umem_reg might be a bit complicated due to not enforcing padding to be zero [0]. Add a couple of things to make it less error-prone: 1. Remove xdp_umem_reg_v2 since its sizeof is the same as xdp_umem_reg 2. Add BUILD_BUG_ON that checks that the size of xdp_umem_reg_v1 is less than xdp_umem_reg; presumably, when we get to v2, there is gonna be a similar line to enforce that sizeof(v2) > sizeof(v1) 3. Add BUILD_BUG_ON to make sure the last field plus its size matches the overall struct size. The intent is to demonstrate that we don't have any lingering padding. 0: https://lore.kernel.org/bpf/ZqI29QE+5JnkdPmE@boxer/T/#me03113f7c2458fd08f3c4114a7a9472ac3646c98 Reported-by: Julian Schindel <mail@xxxxxxxxxxxxxxxx> Cc: Magnus Karlsson <magnus.karlsson@xxxxxxxxx> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> Signed-off-by: Stanislav Fomichev <sdf@xxxxxxxxxxx> --- net/xdp/xsk.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index 7e16336044b2..1140b2a120ca 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -1320,14 +1320,6 @@ struct xdp_umem_reg_v1 { __u32 headroom; }; -struct xdp_umem_reg_v2 { - __u64 addr; /* Start of packet data area */ - __u64 len; /* Length of packet data area */ - __u32 chunk_size; - __u32 headroom; - __u32 flags; -}; - static int xsk_setsockopt(struct socket *sock, int level, int optname, sockptr_t optval, unsigned int optlen) { @@ -1371,10 +1363,19 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname, if (optlen < sizeof(struct xdp_umem_reg_v1)) return -EINVAL; - else if (optlen < sizeof(struct xdp_umem_reg_v2)) - mr_size = sizeof(struct xdp_umem_reg_v1); else if (optlen < sizeof(mr)) - mr_size = sizeof(struct xdp_umem_reg_v2); + mr_size = sizeof(struct xdp_umem_reg_v1); + + BUILD_BUG_ON(sizeof(struct xdp_umem_reg_v1) >= sizeof(struct xdp_umem_reg)); + + /* Make sure the last field of the struct doesn't have + * uninitialized padding. All padding has to be explicit + * and has to be set to zero by the userspace to make + * struct xdp_umem_reg extensible in the future. + */ + BUILD_BUG_ON(offsetof(struct xdp_umem_reg, tx_metadata_len) + + sizeof_field(struct xdp_umem_reg, tx_metadata_len) != + sizeof(struct xdp_umem_reg)); if (copy_from_sockptr(&mr, optval, mr_size)) return -EFAULT; -- 2.45.2
