Skip to content

Commit 1463cd0

Browse files
alobakingregkh
authored andcommitted
xsk: Harden userspace-supplied xdp_desc validation
commit 07ca98f upstream. Turned out certain clearly invalid values passed in xdp_desc from userspace can pass xp_{,un}aligned_validate_desc() and then lead to UBs or just invalid frames to be queued for xmit. desc->len close to ``U32_MAX`` with a non-zero pool->tx_metadata_len can cause positive integer overflow and wraparound, the same way low enough desc->addr with a non-zero pool->tx_metadata_len can cause negative integer overflow. Both scenarios can then pass the validation successfully. This doesn't happen with valid XSk applications, but can be used to perform attacks. Always promote desc->len to ``u64`` first to exclude positive overflows of it. Use explicit check_{add,sub}_overflow() when validating desc->addr (which is ``u64`` already). bloat-o-meter reports a little growth of the code size: add/remove: 0/0 grow/shrink: 2/1 up/down: 60/-16 (44) Function old new delta xskq_cons_peek_desc 299 330 +31 xsk_tx_peek_release_desc_batch 973 1002 +29 xsk_generic_xmit 3148 3132 -16 but hopefully this doesn't hurt the performance much. Fixes: 341ac98 ("xsk: Support tx_metadata_len") Cc: stable@vger.kernel.org # 6.8+ Signed-off-by: Alexander Lobakin <aleksander.lobakin@intel.com> Reviewed-by: Jason Xing <kerneljasonxing@gmail.com> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20251008165659.4141318-1-aleksander.lobakin@intel.com Signed-off-by: Alexei Starovoitov <ast@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent d381de7 commit 1463cd0

File tree

1 file changed

+35
-10
lines changed

1 file changed

+35
-10
lines changed

net/xdp/xsk_queue.h

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -143,42 +143,67 @@ static inline bool xp_unused_options_set(u32 options)
143143
static inline bool xp_aligned_validate_desc(struct xsk_buff_pool *pool,
144144
struct xdp_desc *desc)
145145
{
146-
u64 addr = desc->addr - pool->tx_metadata_len;
147-
u64 len = desc->len + pool->tx_metadata_len;
148-
u64 offset = addr & (pool->chunk_size - 1);
146+
u64 len = desc->len;
147+
u64 addr, offset;
149148

150-
if (!desc->len)
149+
if (!len)
151150
return false;
152151

153-
if (offset + len > pool->chunk_size)
152+
/* Can overflow if desc->addr < pool->tx_metadata_len */
153+
if (check_sub_overflow(desc->addr, pool->tx_metadata_len, &addr))
154+
return false;
155+
156+
offset = addr & (pool->chunk_size - 1);
157+
158+
/*
159+
* Can't overflow: @offset is guaranteed to be < ``U32_MAX``
160+
* (pool->chunk_size is ``u32``), @len is guaranteed
161+
* to be <= ``U32_MAX``.
162+
*/
163+
if (offset + len + pool->tx_metadata_len > pool->chunk_size)
154164
return false;
155165

156166
if (addr >= pool->addrs_cnt)
157167
return false;
158168

159169
if (xp_unused_options_set(desc->options))
160170
return false;
171+
161172
return true;
162173
}
163174

164175
static inline bool xp_unaligned_validate_desc(struct xsk_buff_pool *pool,
165176
struct xdp_desc *desc)
166177
{
167-
u64 addr = xp_unaligned_add_offset_to_addr(desc->addr) - pool->tx_metadata_len;
168-
u64 len = desc->len + pool->tx_metadata_len;
178+
u64 len = desc->len;
179+
u64 addr, end;
169180

170-
if (!desc->len)
181+
if (!len)
171182
return false;
172183

184+
/* Can't overflow: @len is guaranteed to be <= ``U32_MAX`` */
185+
len += pool->tx_metadata_len;
173186
if (len > pool->chunk_size)
174187
return false;
175188

176-
if (addr >= pool->addrs_cnt || addr + len > pool->addrs_cnt ||
177-
xp_desc_crosses_non_contig_pg(pool, addr, len))
189+
/* Can overflow if desc->addr is close to 0 */
190+
if (check_sub_overflow(xp_unaligned_add_offset_to_addr(desc->addr),
191+
pool->tx_metadata_len, &addr))
192+
return false;
193+
194+
if (addr >= pool->addrs_cnt)
195+
return false;
196+
197+
/* Can overflow if pool->addrs_cnt is high enough */
198+
if (check_add_overflow(addr, len, &end) || end > pool->addrs_cnt)
199+
return false;
200+
201+
if (xp_desc_crosses_non_contig_pg(pool, addr, len))
178202
return false;
179203

180204
if (xp_unused_options_set(desc->options))
181205
return false;
206+
182207
return true;
183208
}
184209

0 commit comments

Comments
 (0)