diff mbox series

[net] ibmvnic: Don't reference skb after sending to VIOS

Message ID 20250214155233.235559-1-nnac123@linux.ibm.com (mailing list archive)
State Accepted
Commit bdf5d13aa05ec314d4385b31ac974d6c7e0997c9
Delegated to: Netdev Maintainers
Headers show
Series [net] ibmvnic: Don't reference skb after sending to VIOS | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 11 maintainers not CCed: linuxppc-dev@lists.ozlabs.org maddy@linux.ibm.com andrew+netdev@lunn.ch edumazet@google.com ricklind@linux.ibm.com kuba@kernel.org npiggin@gmail.com mpe@ellerman.id.au christophe.leroy@csgroup.eu naveen@kernel.org pabeni@redhat.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 22 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 3 this patch: 3
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2025-02-15--03-00 (tests: 891)

Commit Message

Nick Child Feb. 14, 2025, 3:52 p.m. UTC
Previously, after successfully flushing the xmit buffer to VIOS,
the tx_bytes stat was incremented by the length of the skb.

It is invalid to access the skb memory after sending the buffer to
the VIOS because, at any point after sending, the VIOS can trigger
an interrupt to free this memory. A race between reading skb->len
and freeing the skb is possible (especially during LPM) and will
result in use-after-free:
 ==================================================================
 BUG: KASAN: slab-use-after-free in ibmvnic_xmit+0x75c/0x1808 [ibmvnic]
 Read of size 4 at addr c00000024eb48a70 by task hxecom/14495
 <...>
 Call Trace:
 [c000000118f66cf0] [c0000000018cba6c] dump_stack_lvl+0x84/0xe8 (unreliable)
 [c000000118f66d20] [c0000000006f0080] print_report+0x1a8/0x7f0
 [c000000118f66df0] [c0000000006f08f0] kasan_report+0x128/0x1f8
 [c000000118f66f00] [c0000000006f2868] __asan_load4+0xac/0xe0
 [c000000118f66f20] [c0080000046eac84] ibmvnic_xmit+0x75c/0x1808 [ibmvnic]
 [c000000118f67340] [c0000000014be168] dev_hard_start_xmit+0x150/0x358
 <...>
 Freed by task 0:
 kasan_save_stack+0x34/0x68
 kasan_save_track+0x2c/0x50
 kasan_save_free_info+0x64/0x108
 __kasan_mempool_poison_object+0x148/0x2d4
 napi_skb_cache_put+0x5c/0x194
 net_tx_action+0x154/0x5b8
 handle_softirqs+0x20c/0x60c
 do_softirq_own_stack+0x6c/0x88
 <...>
 The buggy address belongs to the object at c00000024eb48a00 which
  belongs to the cache skbuff_head_cache of size 224
==================================================================

Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol")
Signed-off-by: Nick Child <nnac123@linux.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Simon Horman Feb. 17, 2025, 4:10 p.m. UTC | #1
On Fri, Feb 14, 2025 at 09:52:33AM -0600, Nick Child wrote:
> Previously, after successfully flushing the xmit buffer to VIOS,
> the tx_bytes stat was incremented by the length of the skb.
> 
> It is invalid to access the skb memory after sending the buffer to
> the VIOS because, at any point after sending, the VIOS can trigger
> an interrupt to free this memory. A race between reading skb->len
> and freeing the skb is possible (especially during LPM) and will
> result in use-after-free:
>  ==================================================================
>  BUG: KASAN: slab-use-after-free in ibmvnic_xmit+0x75c/0x1808 [ibmvnic]
>  Read of size 4 at addr c00000024eb48a70 by task hxecom/14495
>  <...>
>  Call Trace:
>  [c000000118f66cf0] [c0000000018cba6c] dump_stack_lvl+0x84/0xe8 (unreliable)
>  [c000000118f66d20] [c0000000006f0080] print_report+0x1a8/0x7f0
>  [c000000118f66df0] [c0000000006f08f0] kasan_report+0x128/0x1f8
>  [c000000118f66f00] [c0000000006f2868] __asan_load4+0xac/0xe0
>  [c000000118f66f20] [c0080000046eac84] ibmvnic_xmit+0x75c/0x1808 [ibmvnic]
>  [c000000118f67340] [c0000000014be168] dev_hard_start_xmit+0x150/0x358
>  <...>
>  Freed by task 0:
>  kasan_save_stack+0x34/0x68
>  kasan_save_track+0x2c/0x50
>  kasan_save_free_info+0x64/0x108
>  __kasan_mempool_poison_object+0x148/0x2d4
>  napi_skb_cache_put+0x5c/0x194
>  net_tx_action+0x154/0x5b8
>  handle_softirqs+0x20c/0x60c
>  do_softirq_own_stack+0x6c/0x88
>  <...>
>  The buggy address belongs to the object at c00000024eb48a00 which
>   belongs to the cache skbuff_head_cache of size 224
> ==================================================================
> 
> Fixes: 032c5e82847a ("Driver for IBM System i/p VNIC protocol")
> Signed-off-by: Nick Child <nnac123@linux.ibm.com>

Reviewed-by: Simon Horman <horms@kernel.org>
patchwork-bot+netdevbpf@kernel.org Feb. 18, 2025, 1:20 a.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri, 14 Feb 2025 09:52:33 -0600 you wrote:
> Previously, after successfully flushing the xmit buffer to VIOS,
> the tx_bytes stat was incremented by the length of the skb.
> 
> It is invalid to access the skb memory after sending the buffer to
> the VIOS because, at any point after sending, the VIOS can trigger
> an interrupt to free this memory. A race between reading skb->len
> and freeing the skb is possible (especially during LPM) and will
> result in use-after-free:
>  ==================================================================
>  BUG: KASAN: slab-use-after-free in ibmvnic_xmit+0x75c/0x1808 [ibmvnic]
>  Read of size 4 at addr c00000024eb48a70 by task hxecom/14495
>  <...>
>  Call Trace:
>  [c000000118f66cf0] [c0000000018cba6c] dump_stack_lvl+0x84/0xe8 (unreliable)
>  [c000000118f66d20] [c0000000006f0080] print_report+0x1a8/0x7f0
>  [c000000118f66df0] [c0000000006f08f0] kasan_report+0x128/0x1f8
>  [c000000118f66f00] [c0000000006f2868] __asan_load4+0xac/0xe0
>  [c000000118f66f20] [c0080000046eac84] ibmvnic_xmit+0x75c/0x1808 [ibmvnic]
>  [c000000118f67340] [c0000000014be168] dev_hard_start_xmit+0x150/0x358
>  <...>
>  Freed by task 0:
>  kasan_save_stack+0x34/0x68
>  kasan_save_track+0x2c/0x50
>  kasan_save_free_info+0x64/0x108
>  __kasan_mempool_poison_object+0x148/0x2d4
>  napi_skb_cache_put+0x5c/0x194
>  net_tx_action+0x154/0x5b8
>  handle_softirqs+0x20c/0x60c
>  do_softirq_own_stack+0x6c/0x88
>  <...>
>  The buggy address belongs to the object at c00000024eb48a00 which
>   belongs to the cache skbuff_head_cache of size 224
> ==================================================================
> 
> [...]

Here is the summary with links:
  - [net] ibmvnic: Don't reference skb after sending to VIOS
    https://git.kernel.org/netdev/net/c/bdf5d13aa05e

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index e95ae0d39948..0676fc547b6f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2408,6 +2408,7 @@  static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 	dma_addr_t data_dma_addr;
 	struct netdev_queue *txq;
 	unsigned long lpar_rc;
+	unsigned int skblen;
 	union sub_crq tx_crq;
 	unsigned int offset;
 	bool use_scrq_send_direct = false;
@@ -2522,6 +2523,7 @@  static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 	tx_buff->skb = skb;
 	tx_buff->index = bufidx;
 	tx_buff->pool_index = queue_num;
+	skblen = skb->len;
 
 	memset(&tx_crq, 0, sizeof(tx_crq));
 	tx_crq.v1.first = IBMVNIC_CRQ_CMD;
@@ -2614,7 +2616,7 @@  static netdev_tx_t ibmvnic_xmit(struct sk_buff *skb, struct net_device *netdev)
 		netif_stop_subqueue(netdev, queue_num);
 	}
 
-	tx_bytes += skb->len;
+	tx_bytes += skblen;
 	txq_trans_cond_update(txq);
 	ret = NETDEV_TX_OK;
 	goto out;