Re: [PATCH v2] ice: Fix wrong dsn read in ice_adapter_put

[Cyrill Gorcunov <gorcunov@gmail.com>]

On Mon, May 18, 2026 at 01:21:21AM +0300, Cyrill Gorcunov wrote:
> On Sun, May 17, 2026 at 01:53:07PM +0100, Simon Horman wrote:
> ...
> > >  void ice_adapter_put(struct pci_dev *pdev)
> > >  {
> > > +	const struct ice_pf *pf = pci_get_drvdata(pdev);
> > > +	unsigned long index = pf->adapter->index;
> > 
> > Is it possible for pf->adapter to be NULL here if the device was probed in
> > firmware recovery mode?
> > 
> > If the device enters firmware recovery mode during ice_probe(), the driver
> > calls ice_probe_recovery_mode() and skips the ice_adapter_get() allocation.
> > If the device is subsequently unplugged, the memory-mapped read for the
> > firmware state register might return 0.
> > 
> > This would cause ice_is_recovery_mode() to evaluate to false in ice_remove(),
> > allowing the normal teardown sequence to proceed and call ice_adapter_put().
> > Would the unconditional dereference of pf->adapter->index then cause a NULL
> > pointer dereference?
> 
> If we're in recovery mode the ice_remove will not reach ice_adapter_put,
> instead it will stop service work and just deinit devlink interface, no?

Thinking more I think I got what sashiko meant here: the pullout of the
adapter when it been in recovery mode already, and reading register state
is obviously incorrect here too, instead we have to save the state upon
probing in some flag and use it later. I'll update the patch.