On 2/10/2021 3:57 PM, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.olt...@nxp.com>
>
> Due to the fact that ic_dev->dev is kept open in ic_close_dev, I had
> thought that ic_dev will not be freed either. But that is not the case,
> but instead "everybody dies" when ipconfig cleans up, and just the
> net_device behind ic_dev->dev remains allocated but not ic_dev itself.
>
> This is a problem because in ic_close_devs, for every net device that
> we're about to close, we compare it against the list of lower interfaces
> of ic_dev, to figure out whether we should close it or not. But since
> ic_dev itself is subject to freeing, this means that at some point in
> the middle of the list of ipconfig interfaces, ic_dev will have been
> freed, and we would be still attempting to iterate through its list of
> lower interfaces while checking whether to bring down the remaining
> ipconfig interfaces.
>
> There are multiple ways to avoid the use-after-free: we could delay
> freeing ic_dev until the very end (outside the while loop). Or an even
> simpler one: we can observe that we don't need ic_dev when iterating
> through its lowers, only ic_dev->dev, structure which isn't ever freed.
> So, by keeping ic_dev->dev in a variable assigned prior to freeing
> ic_dev, we can avoid all use-after-free issues.
>
> Fixes: 46acf7bdbc72 ("Revert "net: ipv4: handle DSA enabled master network
> devices"")
> Reported-by: kernel test robot <oliver.s...@intel.com>
> Signed-off-by: Vladimir Oltean <vladimir.olt...@nxp.com>
Reviewed-by: Florian Fainelli <f.faine...@gmail.com>
--
Florian
