Re: [PATCH net 1/2] net: pse-pd: disable IRQ before freeing PI data in unregister

[Carlo Szelinsky <github@szelinsky.de>]

Hi Simon,

Thanks for the review.

> pse_flush_pw_ds() runs before disable_irq(), so an interrupt could
> hit a freed regulator.

Correct, and it's the same bug. I moved disable_irq() above
pse_release_pis(), but pse_flush_pw_ds() still runs while the IRQ is
live, and it can free pw_d->supply. The ISR uses that supply via
pse_pi_deallocate_pw_budget(). So the race stays open.

Fix: disable the IRQ (and cancel the poll work) before
pse_flush_pw_ds() too. I'll fold that into patch 1 for v2.

> cancel_work_sync() after pse_release_pis() may use freed pcdev->pi.

I don't think so. The worker only touches the kfifo and the
pse_control list, not pcdev->pi. The patch 1 message says this.
Did I miss a path where the worker reaches pcdev->pi?

> Regulator ops still reachable after pcdev->pi is freed.

That is what patch 2 fixes for the disable path. Are you pointing at
a different path than the regulator_unregister() disable flush?

> device still in the list / external consumers / power domain tied to
> devm lifetime.

These look pre-existing and not part of this series. Do you agree, or
do you see one of them as caused by this series?

The pre-existing items above (list, consumers, devm lifetime) - would
you want them fixed inside this net series, or handled separately on
top? So I know what to do before sending v2.

Thanks,
Carlo