Skip to content

Commit ad51857

Browse files
keesSaeed Mahameed
authored andcommitted
net/mlx5e: Avoid field-overflowing memcpy()
In preparation for FORTIFY_SOURCE performing compile-time and run-time field bounds checking for memcpy(), memmove(), and memset(), avoid intentionally writing across neighboring fields. Use flexible arrays instead of zero-element arrays (which look like they are always overflowing) and split the cross-field memcpy() into two halves that can be appropriately bounds-checked by the compiler. We were doing: #define ETH_HLEN 14 #define VLAN_HLEN 4 ... #define MLX5E_XDP_MIN_INLINE (ETH_HLEN + VLAN_HLEN) ... struct mlx5e_tx_wqe *wqe = mlx5_wq_cyc_get_wqe(wq, pi); ... struct mlx5_wqe_eth_seg *eseg = &wqe->eth; struct mlx5_wqe_data_seg *dseg = wqe->data; ... memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE); target is wqe->eth.inline_hdr.start (which the compiler sees as being 2 bytes in size), but copying 18, intending to write across start (really vlan_tci, 2 bytes). The remaining 16 bytes get written into wqe->data[0], covering byte_count (4 bytes), lkey (4 bytes), and addr (8 bytes). struct mlx5e_tx_wqe { struct mlx5_wqe_ctrl_seg ctrl; /* 0 16 */ struct mlx5_wqe_eth_seg eth; /* 16 16 */ struct mlx5_wqe_data_seg data[]; /* 32 0 */ /* size: 32, cachelines: 1, members: 3 */ /* last cacheline: 32 bytes */ }; struct mlx5_wqe_eth_seg { u8 swp_outer_l4_offset; /* 0 1 */ u8 swp_outer_l3_offset; /* 1 1 */ u8 swp_inner_l4_offset; /* 2 1 */ u8 swp_inner_l3_offset; /* 3 1 */ u8 cs_flags; /* 4 1 */ u8 swp_flags; /* 5 1 */ __be16 mss; /* 6 2 */ __be32 flow_table_metadata; /* 8 4 */ union { struct { __be16 sz; /* 12 2 */ u8 start[2]; /* 14 2 */ } inline_hdr; /* 12 4 */ struct { __be16 type; /* 12 2 */ __be16 vlan_tci; /* 14 2 */ } insert; /* 12 4 */ __be32 trailer; /* 12 4 */ }; /* 12 4 */ /* size: 16, cachelines: 1, members: 9 */ /* last cacheline: 16 bytes */ }; struct mlx5_wqe_data_seg { __be32 byte_count; /* 0 4 */ __be32 lkey; /* 4 4 */ __be64 addr; /* 8 8 */ /* size: 16, cachelines: 1, members: 3 */ /* last cacheline: 16 bytes */ }; So, split the memcpy() so the compiler can reason about the buffer sizes. "pahole" shows no size nor member offset changes to struct mlx5e_tx_wqe nor struct mlx5e_umr_wqe. "objdump -d" shows no meaningful object code changes (i.e. only source line number induced differences and optimizations). Fixes: b5503b9 ("net/mlx5e: XDP TX forwarding support") Signed-off-by: Kees Cook <keescook@chromium.org> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
1 parent 6d5c900 commit ad51857

File tree

2 files changed

+6
-4
lines changed
  • drivers/net/ethernet/mellanox/mlx5/core

2 files changed

+6
-4
lines changed

drivers/net/ethernet/mellanox/mlx5/core/en.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ static inline int mlx5e_get_max_num_channels(struct mlx5_core_dev *mdev)
224224
struct mlx5e_tx_wqe {
225225
struct mlx5_wqe_ctrl_seg ctrl;
226226
struct mlx5_wqe_eth_seg eth;
227-
struct mlx5_wqe_data_seg data[0];
227+
struct mlx5_wqe_data_seg data[];
228228
};
229229

230230
struct mlx5e_rx_wqe_ll {
@@ -241,8 +241,8 @@ struct mlx5e_umr_wqe {
241241
struct mlx5_wqe_umr_ctrl_seg uctrl;
242242
struct mlx5_mkey_seg mkc;
243243
union {
244-
struct mlx5_mtt inline_mtts[0];
245-
struct mlx5_klm inline_klms[0];
244+
DECLARE_FLEX_ARRAY(struct mlx5_mtt, inline_mtts);
245+
DECLARE_FLEX_ARRAY(struct mlx5_klm, inline_klms);
246246
};
247247
};
248248

drivers/net/ethernet/mellanox/mlx5/core/en/xdp.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,8 +341,10 @@ mlx5e_xmit_xdp_frame(struct mlx5e_xdpsq *sq, struct mlx5e_xmit_data *xdptxd,
341341

342342
/* copy the inline part if required */
343343
if (sq->min_inline_mode != MLX5_INLINE_MODE_NONE) {
344-
memcpy(eseg->inline_hdr.start, xdptxd->data, MLX5E_XDP_MIN_INLINE);
344+
memcpy(eseg->inline_hdr.start, xdptxd->data, sizeof(eseg->inline_hdr.start));
345345
eseg->inline_hdr.sz = cpu_to_be16(MLX5E_XDP_MIN_INLINE);
346+
memcpy(dseg, xdptxd->data + sizeof(eseg->inline_hdr.start),
347+
MLX5E_XDP_MIN_INLINE - sizeof(eseg->inline_hdr.start));
346348
dma_len -= MLX5E_XDP_MIN_INLINE;
347349
dma_addr += MLX5E_XDP_MIN_INLINE;
348350
dseg++;

0 commit comments

Comments
 (0)