Re: [Intel-wired-lan] [PATCH] net: ixgbe: Fix memleak in ixgbe_configure_clsu32
From: Paul Menzel <hidden>
Date: 2021-01-03 08:42:02
Also in:
intel-wired-lan, lkml
Dear Dinghao, Am 03.01.21 um 09:08 schrieb Dinghao Liu:
When ixgbe_fdir_write_perfect_filter_82599() fails, input allocated by kzalloc() has not been freed, which leads to memleak.
Nice find. Thank you for your patches. Out of curiosity, do you use an analysis tool to find these issues?
quoted hunk ↗ jump to hunk
Signed-off-by: Dinghao Liu <redacted> --- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 393d1c2cd853..e9c2d28efc81 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c@@ -9582,8 +9582,10 @@ static int ixgbe_configure_clsu32(struct ixgbe_adapter *adapter, ixgbe_atr_compute_perfect_hash_82599(&input->filter, mask); err = ixgbe_fdir_write_perfect_filter_82599(hw, &input->filter, input->sw_idx, queue); - if (!err) - ixgbe_update_ethtool_fdir_entry(adapter, input, input->sw_idx); + if (err) + goto err_out_w_lock; + + ixgbe_update_ethtool_fdir_entry(adapter, input, input->sw_idx); spin_unlock(&adapter->fdir_perfect_lock); if ((uhtid != 0x800) && (adapter->jump_tables[uhtid]))
Reviewed-by: Paul Menzel <redacted> I wonder, in the non-error case, how `input` and `jump` are freed. Old code:
if (!err)
ixgbe_update_ethtool_fdir_entry(adapter, input, input->sw_idx);
spin_unlock(&adapter->fdir_perfect_lock);
if ((uhtid != 0x800) && (adapter->jump_tables[uhtid]))
set_bit(loc - 1, (adapter->jump_tables[uhtid])->child_loc_map);
kfree(mask);
return err;Should these two lines be replaced with `goto err_out`? (Though the name is confusing then, as it’s the non-error case.)
err_out_w_lock:
spin_unlock(&adapter->fdir_perfect_lock);
err_out:
kfree(mask);
free_input:
kfree(input);
free_jump:
kfree(jump);
return err;
}Kind regards, Paul