Re: [PATCH iwl-net 6/6] ice: register LAG event handler last in ice_init_lag()

[Jacob Keller <jacob.e.keller@intel.com>]

On 11/3/2026 4:42 AM, Przemek Kitszel wrote:
> On 11/3/26 12:23 PM, Loktionov, Aleksandr wrote:
>>
>>
>>> -----Original Message-----
>>> From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> Sent: Monday, November 2, 2026 11:50 AM
>>> To: intel-wired-lan@lists.osuosl.org; Oros, Petr <poros@redhat.com>
>>> Cc: netdev@vger.kernel.org; Kitszel, Przemyslaw
>>> <przemyslaw.kitszel@intel.com>; Sashiko <sashiko-bot@kernel.org>
>>> Subject: [PATCH iwl-net 6/6] ice: register LAG event handler last in
>>> ice_init_lag()
>>>
>>> ice_init_lag() registers the netdevice notifier before it creates the
>>> LAG recipes, but when creating any of them fails, the error path frees
>>> the lag struct without unregistering the notifier. A failure of
>>> ice_init_lag() is not fatal to probe, and pf->lag is set to NULL, so
>>> ice_deinit_lag() won't unregister it later either. The next netdev event
>>> then makes
>>> ice_lag_event_handler() use the freed lag struct.
>>>
>>> Register the notifier last, after the recipes are created and associated
>>> with the profiles, so the error paths have no handler, nor queued work
>>> for it, that could use the lag struct or its recipes. Free the recipes
>>> if the registration itself fails.
>>>
>>> Fixes: ec5a6c5f79ed ("ice: process events created by lag netdev event
>>> handler")
>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>> Closes: https://sashiko.dev/#/bug/linux-e0559bca-fcfd-4435-96f2-
>>> 15f7e3d74184
>>> Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>>> ---
>>>   drivers/net/ethernet/intel/ice/ice_lag.c | 15 +++++++++------
>>>   1 file changed, 9 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/intel/ice/ice_lag.c
>>> b/drivers/net/ethernet/intel/ice/ice_lag.c
>>> index 08a17ded0ad5..c4b8af01e2af 100644
>>> --- a/drivers/net/ethernet/intel/ice/ice_lag.c
>>> +++ b/drivers/net/ethernet/intel/ice/ice_lag.c
>>> @@ -2601,12 +2601,6 @@ int ice_init_lag(struct ice_pf *pf)
>>>       lag->notif_block.notifier_call = NULL;
>>>       memset(lag->sec_vf, 0, sizeof(lag->sec_vf));
>>>
>>> -    err = ice_register_lag_handler(lag);
>>> -    if (err) {
>>> -        dev_warn(dev, "INIT LAG: Failed to register event
>>> handler\n");
>>> -        goto lag_error;
>>> -    }
>>> -
>>>       err = ice_create_lag_recipe(&pf->hw, &lag->pf_recipe,
>>>                       ice_dflt_vsi_rcp, 1);
>> I'm afraid act_act_recipe is allocated but never released on the
>> normal teardown path.
>> See ice_deinit_lag() it has just kfree(lag);
> 
> you are right, the code behaves that way both before and after the patch
> FW resets those via PFR at appropriate moment (appropriate to ensure
> correctness, not to have clean code).
> 
> Not going to change that as part of little fix.

Yep, that should be cleaned up as a separate from this series. Will
apply this to dev-queue.