[net,0/2] ibmvnic: Fix TX skb leak after device reset

Message

Nick Child June 20, 2024, 3:23 p.m. UTC

These 2 patches focus on resolving a possible skb leak after
a subset of the ibmvnic reset processes.

Essentially, the driver maintains a free_map which contains indexes to a
list of tracked skb's addresses on xmit. Due to a mistake during reset,
the free_map did not accurately map to free indexes in the skb list.
This resulted in a leak in skb because the index in free_map was blindly
trusted to contain a NULL pointer. So this patchset addresses 2 issues:
  1. We shouldn't blindly trust our free_map (lets not do this again)
  2. We need to ensure that our free_map is accurate in the first place

The first patch is more cautionary to detect these leaks in any future
bugs (while also helping to justify the leak fixed in the second patch).
In this case it is due to device resets which free the tx complete irq
but do not free the outstanding skb's which would have been freed by the
irq handler ibmvnic_complete_tx().

These outstanding SKB's MUST be freed any time we free the IRQ. We are
not going to get an IRQ to free them later on! Also, further in the
reset path init_tx_pools() is going to mark all buffers free! This is
addressed by the second patch.

Nick Child (2):
  ibmvnic: Add tx check to prevent skb leak
  ibmvnic: Free any outstanding tx skbs during scrq reset

 drivers/net/ethernet/ibm/ibmvnic.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

Comments

[e-mail] June 22, 2024, 10:40 a.m. UTC | #1

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <[e-mail]>:

On Thu, 20 Jun 2024 10:23:10 -0500 you wrote:
> These 2 patches focus on resolving a possible skb leak after
> a subset of the ibmvnic reset processes.
> 
> Essentially, the driver maintains a free_map which contains indexes to a
> list of tracked skb's addresses on xmit. Due to a mistake during reset,
> the free_map did not accurately map to free indexes in the skb list.
> This resulted in a leak in skb because the index in free_map was blindly
> trusted to contain a NULL pointer. So this patchset addresses 2 issues:
>   1. We shouldn't blindly trust our free_map (lets not do this again)
>   2. We need to ensure that our free_map is accurate in the first place
> 
> [...]

Here is the summary with links:
  - [net,1/2] ibmvnic: Add tx check to prevent skb leak
    (no matching commit)
  - [net,2/2] ibmvnic: Free any outstanding tx skbs during scrq reset
    https://git.kernel.org/netdev/net/c/49bbeb5719c2

You are awesome, thank you!

[e-mail] June 25, 2024, 9:10 a.m. UTC | #2

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <[e-mail]>:

On Thu, 20 Jun 2024 10:23:10 -0500 you wrote:
> These 2 patches focus on resolving a possible skb leak after
> a subset of the ibmvnic reset processes.
> 
> Essentially, the driver maintains a free_map which contains indexes to a
> list of tracked skb's addresses on xmit. Due to a mistake during reset,
> the free_map did not accurately map to free indexes in the skb list.
> This resulted in a leak in skb because the index in free_map was blindly
> trusted to contain a NULL pointer. So this patchset addresses 2 issues:
>   1. We shouldn't blindly trust our free_map (lets not do this again)
>   2. We need to ensure that our free_map is accurate in the first place
> 
> [...]

Here is the summary with links:
  - [net,1/2] ibmvnic: Add tx check to prevent skb leak
    https://git.kernel.org/netdev/net/c/0983d288caf9
  - [net,2/2] ibmvnic: Free any outstanding tx skbs during scrq reset
    (no matching commit)

You are awesome, thank you!