|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [PATCH net-next 0/9] net: stop calling __pskb_pull_tail() from drivers
__pskb_pull_tail() is the slow path behind pskb_may_pull() and __skb_linearize(), and drivers shouldn't be calling it directly. It takes the number of bytes to pull relative to the current head and does no bounds checking on it. Ask for more than the skb holds and it BUG()s in skb_copy_bits(). Ask for a negative amount, which is what a caller computing "len - skb_headlen(skb)" gets once the head is already long enough, and skb_copy_bits() is handed a length of nearly 4GB to copy into the head. Under KASAN that shows up as an out-of-bounds read of size 4294967288, after which __pskb_pull_tail() returns success with the skb's head and paged lengths no longer matching its frags. It also returns NULL on failure with nothing making the caller look at it. Eight network drivers call it directly and four of them don't check. All four are RX paths where a failed pull is followed by eth_type_trans() or skb_pull(), which BUG() in __skb_pull() once the head is shorter than what they pull. As far as I can tell none of the four can fail today, since the skb is fresh, isn't shared and has room in the head, but that only holds because of how each driver happens to allocate. pskb_may_pull() and __skb_linearize() take the length the head should end up with, check it against skb->len and fail cleanly, which is what every one of these callers wants. Convert all eight drivers to them, check the result, and drop the packet on failure. The last patch makes skb_condense() check its pull as well. It can't fail there today, but it would undercount truesize if it ever did. The callers left in aoe and xen-netfront are fixed separately, through the block and net trees: https://lore.kernel.org/r/20261007-b4-aoe-short-packets-v1-1-db5155f7bb9c@xxxxxxxxxxxxxx https://lore.kernel.org/r/20261007-b4-xen-netfront-short-head-v1-1-12d7113a7e4e@xxxxxxxxxxxxxx Once those are in I'd like to stop exporting __pskb_pull_tail() so new drivers can't pick it up. Testing: every touched file builds with W=1 on x86_64 allmodconfig (ftmac100 on i386, it's 32-bit only). e1000e under QEMU, which hits the converted TSO workaround on every TSO packet, passes TCP traffic between two emulated 82574Ls, and with failslab failing 5% of atomic allocations the failed pulls drop the packet and nothing falls over. The same setup with jumbo frames and copybreak off makes skb_condense() pull frag data into the head tens of thousands of times a run. xen-netfront ran as a Xen HVM guest under QEMU's KVM Xen emulation, with the backend changed to spread frames over 18 and 19 RX slots, which takes the converted pull in xennet_fill_frags() and its overflow path. The other drivers are compile tested only. Thanks, Josef --- Josef Bacik (9): net: ftmac100: check for failure when pulling in the RX header net/mlx5e: check for failure when pulling the Ethernet header after XDP net: niu: check for failure when pulling in the RX header xen/netfront: check for failure when pulling in xennet_fill_frags() e1000: use pskb_may_pull() in the 82544 TSO workaround e1000e: use pskb_may_pull() in the 82571/2/3 TSO workaround netxen: use pskb_may_pull() to pull excess TX frags into the head qlcnic: use pskb_may_pull() to pull excess TX frags into the head net: skbuff: don't reset truesize in skb_condense() if the pull fails drivers/net/ethernet/faraday/ftmac100.c | 10 ++++++++-- drivers/net/ethernet/intel/e1000/e1000_main.c | 5 ++--- drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++-- drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 9 ++++++--- .../net/ethernet/qlogic/netxen/netxen_nic_main.c | 2 +- drivers/net/ethernet/qlogic/qlcnic/qlcnic_io.c | 2 +- drivers/net/ethernet/sun/niu.c | 6 +++++- drivers/net/xen-netfront.c | 22 ++++++++++------------ net/core/skbuff.c | 3 ++- 9 files changed, 37 insertions(+), 26 deletions(-) --- base-commit: a5e7d8e446af9803e37a3b6a4d416fb41178348f change-id: 20261006-b4-pskb-pull-tail-drivers-ff4fb2964e1a
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |