[PATCH net 00/10] net: pse-pd: assorted lifetime and error-path fixes

[PATCH net 00/10] net: pse-pd: assorted lifetime and error-path fixes

[Carlo Szelinsky]

Fixes for the PSE core and two of its drivers, found while working on
the PSE controller teardown. Two patches depend on the one before them:
6 is only safe with the locked check from 5, and 8 relies on 7 to give
back a budget that was already reserved.

 1  The kfifo is too small: podl-pse-regulator and any controller with
    0 or 1 lines cannot register.
 2  A module reference leaks when reading the port state fails.
 3  A failed IRQ request stays in pcdev->irq, so unregister disables a
    line another device may own.
 4  The notification worker can read a PHY that was already freed.
 5  Setting a power limit checks the PI state without the lock.
 6  A power domain shared by two controllers is freed when the one that
    created it unbinds.
 7  pd692x0 leaks its manager power budget when probe fails.
 8  pd692x0 crashes on a manager supply that is not resolved yet.
 9  si3474 logs through an I2C client it has already unregistered.
 10 A low voltage reading overflows the current for a power limit and
    hits a BUG_ON().

Testing: every patch builds on its own with W=1 and is checkpatch
clean. 1, 4 and 6 were tested in QEMU (arm64, KASAN, lockdep, kmemleak)
with a test-only fake PSE controller: net/main shows the kfifo failure
and use-after-free reports, the series is clean. The other patches are
build-tested only. Together with the PSE core fixes and the decouple
series, the series also runs on a Hasivo S600WP (hs104), including
setting power limits.

Pending work:
 - "net: pse-pd: fix use-after-free of PI array on controller
   unregister" [1] applies with this series in either order.
 - The PSE core fixes for net-next [2] conflict with 5 and 6; the note
   in their patch 2 explains how to merge them.

Not fixed here: a shared power domain's supply handle belongs to the PI
regulator of the controller that created it, so it goes stale when
that controller unbinds. That is older than this series.

[1] https://lore.kernel.org/netdev/20260813200653.980170-1-github@szelinsky.de/
[2] https://lore.kernel.org/netdev/20261018081853.47484-1-github@szelinsky.de/

Carlo Szelinsky (10):
  net: pse-pd: size the notification kfifo after defaulting nr_lines
  net: pse-pd: fix module refcount leak in pse_control_get_internal()
  net: pse-pd: do not keep a failed IRQ in pcdev->irq
  net: pse-pd: do not reach a freed PHY from the notification worker
  net: pse-pd: check the PI state under the lock when setting a power
    limit
  net: pse-pd: do not tie a shared power domain to its creator's devres
  net: pse-pd: pd692x0: release the manager power budget through devres
  net: pse-pd: pd692x0: defer probe while a manager supply is unresolved
  net: pse-pd: si3474: fix use-after-free in ancillary client error path
  net: pse-pd: reject a power limit whose current does not fit an int

 drivers/net/pse-pd/pd692x0.c  |  31 ++++--
 drivers/net/pse-pd/pse_core.c | 171 +++++++++++++++++++++++-----------
 drivers/net/pse-pd/si3474.c   |   4 +-
 3 files changed, 140 insertions(+), 66 deletions(-)


base-commit: aaaaf87ea99b8766c9a8aa0e71aa42e6bc8a5320
-- 
2.43.0

[PATCH net 01/10] net: pse-pd: size the notification kfifo after defaulting nr_lines

[Carlo Szelinsky]

pse_controller_register() allocates the notification kfifo before it
turns nr_lines == 0 into 1, and kfifo_alloc() fails for a size below 2.
So a controller with nr_lines 0 or 1 cannot register. pse_regulator
(podl-pse-regulator) never sets nr_lines, so its probe has failed with
-EINVAL ever since the kfifo was added.

Apply the default first, and allocate at least two entries.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index a5e6d7b26b9f..c0573cbbba5b 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1044,16 +1044,19 @@ int pse_controller_register(struct pse_controller_dev *pcdev)
 	mutex_init(&pcdev->lock);
 	INIT_LIST_HEAD(&pcdev->pse_control_head);
 	spin_lock_init(&pcdev->ntf_fifo_lock);
-	ret = kfifo_alloc(&pcdev->ntf_fifo, pcdev->nr_lines, GFP_KERNEL);
+
+	if (!pcdev->nr_lines)
+		pcdev->nr_lines = 1;
+
+	/* kfifo_alloc() rejects a size that rounds up to less than 2 */
+	ret = kfifo_alloc(&pcdev->ntf_fifo, max(pcdev->nr_lines, 2U),
+			  GFP_KERNEL);
 	if (ret) {
 		dev_err(pcdev->dev, "failed to allocate kfifo notifications\n");
 		return ret;
 	}
 	INIT_WORK(&pcdev->ntf_work, pse_send_ntf_worker);
 
-	if (!pcdev->nr_lines)
-		pcdev->nr_lines = 1;
-
 	if (!pcdev->ops->pi_get_admin_state ||
 	    !pcdev->ops->pi_get_pw_status) {
 		dev_err(pcdev->dev,
-- 
2.43.0

[PATCH net 02/10] net: pse-pd: fix module refcount leak in pse_control_get_internal()

[Carlo Szelinsky]

pse_control_get_internal() takes a module reference and then has two
error paths that skip module_put(): a controller without
pi_get_admin_state(), and a failing pse_pi_is_hw_enabled(). The second
one happens on any I2C or UART error. Each time, one more reference
leaks, and the PSE driver can no longer be unloaded.

Send both to put_module, like the regulator_get_exclusive() error path.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index c0573cbbba5b..1710f9bb7d27 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1427,7 +1427,7 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
 
 	if (!pcdev->ops->pi_get_admin_state) {
 		ret = -EOPNOTSUPP;
-		goto free_psec;
+		goto put_module;
 	}
 
 	/* Initialize admin_state_enabled before the regulator_get. This
@@ -1436,7 +1436,7 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
 	 */
 	ret = pse_pi_is_hw_enabled(pcdev, index);
 	if (ret < 0)
-		goto free_psec;
+		goto put_module;
 
 	pcdev->pi[index].admin_state_enabled = ret;
 	psec->ps = regulator_get_exclusive(pcdev->dev,
-- 
2.43.0

[PATCH net 03/10] net: pse-pd: do not keep a failed IRQ in pcdev->irq

[Carlo Szelinsky]

devm_pse_irq_helper() keeps the IRQ number in pcdev->irq even when the
IRQ request failed. The failed probe then runs
pse_controller_unregister(), which calls disable_irq() on that line. If
another device owns it, that device stops getting interrupts.

Clear pcdev->irq when the request fails. It is now set before the
request, because pse_isr() can run as soon as the IRQ is enabled and
pse_pw_d_is_sw_pw_control() reads it.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 1710f9bb7d27..65a41c517b35 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1348,14 +1348,20 @@ int devm_pse_irq_helper(struct pse_controller_dev *pcdev, int irq,
 	if (!h->notifs_mask)
 		return -ENOMEM;
 
+	/* pse_isr() can run as soon as the IRQ is enabled, and
+	 * pse_pw_d_is_sw_pw_control() reads pcdev->irq.
+	 */
+	pcdev->irq = irq;
 	ret = devm_request_threaded_irq(dev, irq, NULL, pse_isr,
 					IRQF_ONESHOT | irq_flags,
 					irq_name, h);
-	if (ret)
+	if (ret) {
+		pcdev->irq = 0;
 		dev_err(pcdev->dev, "Failed to request IRQ %d\n", irq);
+		return ret;
+	}
 
-	pcdev->irq = irq;
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL_GPL(devm_pse_irq_helper);
 
-- 
2.43.0

[PATCH net 04/10] net: pse-pd: do not reach a freed PHY from the notification worker

[Carlo Szelinsky]

The notification worker holds a reference on the pse_control, but not
on the PHY that psec->attached_phydev points to. If the PHY is removed
while the worker runs, the pse_control survives and the worker reads the
freed PHY. The same happens without a race when two PHYs share one PI:
attached_phydev keeps pointing at the first PHY after it is removed.

pse_control_put() unlinks and frees a pse_control under pse_list_mutex,
and phy_device_remove() calls it before the PHY is freed. So instead of
taking a reference, let the worker look the pse_control up and use
attached_phydev under pse_list_mutex. This nests pse_list_mutex inside
rtnl; nothing under pse_list_mutex takes rtnl.

For a PI shared by two PHYs there is no single netdev to report to, so
clear attached_phydev when a second PHY takes the pse_control, and warn.
Events for that PI are then no longer reported, even after one PHY goes
away. Failing the second PHY instead would fail its whole MDIO bus.

Fixes: fa2f0454174c ("net: pse-pd: Introduce attached_phydev to pse control")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 42 ++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 65a41c517b35..336f00d82b5f 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -238,24 +238,22 @@ static int of_load_pse_pis(struct pse_controller_dev *pcdev)
  * @pcdev: a pointer to the PSE
  * @id: index of the PSE control
  *
- * Return: pse_control pointer or NULL. The device returned has had a
- *	   reference added and the pointer is safe until the user calls
- *	   pse_control_put() to indicate they have finished with it.
+ * Must be called with pse_list_mutex held. No reference is taken: the
+ * pointer is only valid until pse_list_mutex is released.
+ *
+ * Return: pse_control pointer or NULL.
  */
 static struct pse_control *
 pse_control_find_by_id(struct pse_controller_dev *pcdev, int id)
 {
 	struct pse_control *psec;
 
-	mutex_lock(&pse_list_mutex);
+	lockdep_assert_held(&pse_list_mutex);
+
 	list_for_each_entry(psec, &pcdev->pse_control_head, list) {
-		if (psec->id == id) {
-			kref_get(&psec->refcnt);
-			mutex_unlock(&pse_list_mutex);
+		if (psec->id == id)
 			return psec;
-		}
 	}
-	mutex_unlock(&pse_list_mutex);
 	return NULL;
 }
 
@@ -268,6 +266,7 @@ pse_control_find_by_id(struct pse_controller_dev *pcdev, int id)
 static struct net_device *pse_control_get_netdev(struct pse_control *psec)
 {
 	ASSERT_RTNL();
+	lockdep_assert_held(&pse_list_mutex);
 
 	if (!psec || !psec->attached_phydev)
 		return NULL;
@@ -1020,13 +1019,21 @@ static void pse_send_ntf_worker(struct work_struct *work)
 		struct net_device *netdev;
 		struct pse_control *psec;
 
-		psec = pse_control_find_by_id(pcdev, ntf.id);
+		/* A pse_control is only on the list while the PHY it was
+		 * handed to holds it: the PHY's pse_control_put() unlinks
+		 * and frees it under pse_list_mutex before the PHY itself
+		 * can be freed. Holding the mutex across the lookup and the
+		 * attached_phydev dereference therefore keeps the PHY alive,
+		 * which a reference on the pse_control alone would not.
+		 */
 		rtnl_lock();
+		mutex_lock(&pse_list_mutex);
+		psec = pse_control_find_by_id(pcdev, ntf.id);
 		netdev = pse_control_get_netdev(psec);
 		if (netdev)
 			ethnl_pse_send_ntf(netdev, ntf.notifs);
+		mutex_unlock(&pse_list_mutex);
 		rtnl_unlock();
-		pse_control_put(psec);
 	}
 }
 
@@ -1417,6 +1424,19 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
 
 	list_for_each_entry(psec, &pcdev->pse_control_head, list) {
 		if (psec->id == index) {
+			/* attached_phydev can only name one PHY. If a second
+			 * one shares the PI, either may be removed first and
+			 * leave it pointing at a freed phy_device, so stop
+			 * reporting events for this PI instead.
+			 */
+			if (psec->attached_phydev &&
+			    psec->attached_phydev != phydev) {
+				dev_warn(pcdev->dev,
+					 "PI %u shared by several PHYs, not reporting its events\n",
+					 index);
+				psec->attached_phydev = NULL;
+			}
+
 			kref_get(&psec->refcnt);
 			return psec;
 		}
-- 
2.43.0

[PATCH net 05/10] net: pse-pd: check the PI state under the lock when setting a power limit

[Carlo Szelinsky]

pse_ethtool_set_pw_limit() checks whether the PI is powered without
pcdev->lock. pse_isr() changes that state under the lock, so if the PD
disconnects in between, budget is reserved for a port that is no longer
powered.

Do the check in pse_pi_update_pw_budget() under the lock, and report
whether the budget was changed, so the rollback after a failed
regulator_set_current_limit() runs only then. This also rolls back a
budget that started from zero, which the old code skipped.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 336f00d82b5f..d5f863834179 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1829,22 +1829,32 @@ EXPORT_SYMBOL_GPL(pse_ethtool_set_config);
  * @pcdev: a pointer to the PSE controller device
  * @id: index of the PSE PI
  * @pw_req: power requested
+ * @updated: set to true if the budget was changed
  * @extack: extack for reporting useful error messages
  *
  * Return: Previous power allocated on success and failure value on error
  */
 static int pse_pi_update_pw_budget(struct pse_controller_dev *pcdev, int id,
-				   const unsigned int pw_req,
+				   const unsigned int pw_req, bool *updated,
 				   struct netlink_ext_ack *extack)
 {
 	struct pse_pi *pi = &pcdev->pi[id];
 	int previous_pw_allocated;
 	int pw_diff, ret = 0;
 
+	*updated = false;
+
 	/* We don't want pw_allocated_mW value change in the middle of an
 	 * power budget update
 	 */
 	mutex_lock(&pcdev->lock);
+	/* Only a powered PI under software power control holds a budget.
+	 * Check it here, as pse_isr() changes it under the same lock.
+	 */
+	if (!pse_pw_d_is_sw_pw_control(pcdev, pi->pw_d) ||
+	    !pi->admin_state_enabled || !pi->isr_pd_detected)
+		goto out;
+
 	previous_pw_allocated = pi->pw_allocated_mW;
 	pw_diff = pw_req - previous_pw_allocated;
 	if (!pw_diff) {
@@ -1862,6 +1872,7 @@ static int pse_pi_update_pw_budget(struct pse_controller_dev *pcdev, int id,
 		regulator_free_power_budget(pi->pw_d->supply, -pw_diff);
 	}
 	pi->pw_allocated_mW = pw_req;
+	*updated = true;
 	ret = previous_pw_allocated;
 
 out:
@@ -1882,6 +1893,7 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
 			     const unsigned int pw_limit)
 {
 	int uV, uA, ret, previous_pw_allocated = 0;
+	bool updated;
 	s64 tmp_64;
 
 	if (pw_limit > MAX_PI_PW)
@@ -1908,21 +1920,17 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
 	/* Update power budget only in software power control case and
 	 * if a Power Device is powered.
 	 */
-	if (pse_pw_d_is_sw_pw_control(psec->pcdev,
-				      psec->pcdev->pi[psec->id].pw_d) &&
-	    psec->pcdev->pi[psec->id].admin_state_enabled &&
-	    psec->pcdev->pi[psec->id].isr_pd_detected) {
-		ret = pse_pi_update_pw_budget(psec->pcdev, psec->id,
-					      pw_limit, extack);
-		if (ret < 0)
-			return ret;
-		previous_pw_allocated = ret;
-	}
+	ret = pse_pi_update_pw_budget(psec->pcdev, psec->id, pw_limit,
+				      &updated, extack);
+	if (ret < 0)
+		return ret;
+	previous_pw_allocated = ret;
 
 	ret = regulator_set_current_limit(psec->ps, 0, uA);
-	if (ret < 0 && previous_pw_allocated) {
+	if (ret < 0 && updated) {
 		pse_pi_update_pw_budget(psec->pcdev, psec->id,
-					previous_pw_allocated, extack);
+					previous_pw_allocated, &updated,
+					extack);
 	}
 
 	return ret;
-- 
2.43.0

[PATCH net 06/10] net: pse-pd: do not tie a shared power domain to its creator's devres

[Carlo Szelinsky]

A power domain is shared by all controllers on the same supply and
counted with a kref, but it is devm memory of the controller that
created it. When that controller unbinds while another one still uses
the domain, devres frees it anyway, and the other controller and the
global map keep pointing at freed memory. The same happens when
pse_register_pw_ds() fails part way.

Allocate the domain with kzalloc() and free it on its last kref_put().
A domain left over from a failed pse_register_pw_ds() is now leaked
instead of freed while still in the map.

Because the domain can now be freed at the put, pse_flush_pw_ds() clears
pi[].pw_d under pcdev->lock before the put, and pse_ethtool_set_prio()
checks it under that lock. The PI regulator ops and pse_isr() use it
under the same lock.

Fixes: 50f8b341d268 ("net: pse-pd: Add support for PSE power domains")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 43 ++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index d5f863834179..0d1e5d5039f8 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -886,6 +886,7 @@ static void __pse_pw_d_release(struct kref *kref)
 	regulator_put(pw_d->supply);
 	xa_erase(&pse_pw_d_map, pw_d->id);
 	mutex_unlock(&pse_pw_d_mutex);
+	kfree(pw_d);
 }
 
 /**
@@ -898,10 +899,18 @@ static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
 	int i;
 
 	for (i = 0; i < pcdev->nr_lines; i++) {
-		if (!pcdev->pi[i].pw_d)
+		/* Clear it before the put that may free the domain. The PI
+		 * regulator ops and pse_isr() hold pcdev->lock while they
+		 * use it.
+		 */
+		mutex_lock(&pcdev->lock);
+		pw_d = pcdev->pi[i].pw_d;
+		pcdev->pi[i].pw_d = NULL;
+		mutex_unlock(&pcdev->lock);
+		if (!pw_d)
 			continue;
 
-		pw_d = xa_load(&pse_pw_d_map, pcdev->pi[i].pw_d->id);
+		pw_d = xa_load(&pse_pw_d_map, pw_d->id);
 		if (!pw_d)
 			continue;
 
@@ -911,24 +920,30 @@ static void pse_flush_pw_ds(struct pse_controller_dev *pcdev)
 }
 
 /**
- * devm_pse_alloc_pw_d - allocate a new PSE power domain for a device
- * @dev: device that is registering this PSE power domain
+ * pse_alloc_pw_d - allocate a new PSE power domain
+ *
+ * A power domain is shared by every controller whose PIs sit on the same
+ * supply, so it must not be tied to the lifetime of the controller that
+ * happened to create it. It is freed by __pse_pw_d_release() on the last
+ * kref_put().
  *
  * Return: Pointer to the newly allocated PSE power domain or error pointers
  */
-static struct pse_power_domain *devm_pse_alloc_pw_d(struct device *dev)
+static struct pse_power_domain *pse_alloc_pw_d(void)
 {
 	struct pse_power_domain *pw_d;
 	int index, ret;
 
-	pw_d = devm_kzalloc(dev, sizeof(*pw_d), GFP_KERNEL);
+	pw_d = kzalloc_obj(*pw_d);
 	if (!pw_d)
 		return ERR_PTR(-ENOMEM);
 
 	ret = xa_alloc(&pse_pw_d_map, &index, pw_d, XA_LIMIT(1, PSE_PW_D_LIMIT),
 		       GFP_KERNEL);
-	if (ret)
+	if (ret) {
+		kfree(pw_d);
 		return ERR_PTR(ret);
+	}
 
 	kref_init(&pw_d->refcnt);
 	pw_d->id = index;
@@ -974,7 +989,7 @@ static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
 			continue;
 		}
 
-		pw_d = devm_pse_alloc_pw_d(pcdev->dev);
+		pw_d = pse_alloc_pw_d();
 		if (IS_ERR(pw_d)) {
 			ret = PTR_ERR(pw_d);
 			goto out;
@@ -983,6 +998,7 @@ static int pse_register_pw_ds(struct pse_controller_dev *pcdev)
 		supply = regulator_get(&rdev->dev, rdev->supply_name);
 		if (IS_ERR(supply)) {
 			xa_erase(&pse_pw_d_map, pw_d->id);
+			kfree(pw_d);
 			ret = PTR_ERR(supply);
 			goto out;
 		}
@@ -1954,15 +1970,16 @@ int pse_ethtool_set_prio(struct pse_control *psec,
 	const struct pse_controller_ops *ops;
 	int ret = 0;
 
-	if (!pcdev->pi[psec->id].pw_d) {
-		NL_SET_ERR_MSG(extack, "no power domain attached");
-		return -EOPNOTSUPP;
-	}
-
 	/* We don't want priority change in the middle of an
 	 * enable/disable call or a priority mode change
 	 */
 	mutex_lock(&pcdev->lock);
+	if (!pcdev->pi[psec->id].pw_d) {
+		NL_SET_ERR_MSG(extack, "no power domain attached");
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
+
 	switch (pcdev->pi[psec->id].pw_d->budget_eval_strategy) {
 	case PSE_BUDGET_EVAL_STRAT_STATIC:
 		if (prio >= pcdev->nr_lines) {
-- 
2.43.0

[PATCH net 07/10] net: pse-pd: pd692x0: release the manager power budget through devres

[Carlo Szelinsky]

pd692x0 reserves a power budget on each manager's supply in
setup_pi_matrix(), but gives it back only on that function's own errors
and in remove(). If probe fails later, or the request fails for one
manager after others succeeded, the budget stays reserved on vmain. Each
probe retry, e.g. after -EPROBE_DEFER, reserves it again until nothing
is left.

Release the budget from a devm action instead, and drop the explicit
release calls.

Fixes: 1c67f9c54cdc ("net: pse-pd: pd692x0: Fix power budget leak in manager setup error path")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pd692x0.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 209de9cec849..34f0dfbccd06 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -1224,8 +1224,9 @@ static void pd692x0_of_put_managers(struct pd692x0_priv *priv,
 	}
 }
 
-static void pd692x0_managers_free_pw_budget(struct pd692x0_priv *priv)
+static void pd692x0_managers_free_pw_budget(void *data)
 {
+	struct pd692x0_priv *priv = data;
 	int i;
 
 	for (i = 0; i < PD692X0_MAX_MANAGERS; i++) {
@@ -1279,31 +1280,39 @@ static int pd692x0_setup_pi_matrix(struct pse_controller_dev *pcdev)
 	if (ret)
 		goto err_of_managers;
 
+	/* Frees whatever pd692x0_req_managers_pw_budget() manages to
+	 * request, on any later probe failure and on unbind. Added after
+	 * the manager regulators so that it runs before devres unregisters
+	 * them and their supply goes away.
+	 */
+	ret = devm_add_action_or_reset(&priv->client->dev,
+				       pd692x0_managers_free_pw_budget, priv);
+	if (ret)
+		goto err_of_managers;
+
 	ret = pd692x0_req_managers_pw_budget(priv);
 	if (ret)
 		goto err_of_managers;
 
 	ret = pd692x0_set_ports_matrix(priv, manager);
 	if (ret)
-		goto err_managers_req_pw;
+		goto err_of_managers;
 
 	/* Do not init the conf if it is already saved */
 	if (!priv->cfg_saved) {
 		ret = pd692x0_hw_conf_init(priv);
 		if (ret)
-			goto err_managers_req_pw;
+			goto err_of_managers;
 
 		ret = pd692x0_save_user_byte(priv);
 		if (ret)
-			goto err_managers_req_pw;
+			goto err_of_managers;
 	}
 
 	pd692x0_of_put_managers(priv, manager);
 	kfree(manager);
 	return 0;
 
-err_managers_req_pw:
-	pd692x0_managers_free_pw_budget(priv);
 err_of_managers:
 	pd692x0_of_put_managers(priv, manager);
 err_free_manager:
@@ -1855,7 +1864,6 @@ static void pd692x0_i2c_remove(struct i2c_client *client)
 {
 	struct pd692x0_priv *priv = i2c_get_clientdata(client);
 
-	pd692x0_managers_free_pw_budget(priv);
 	firmware_upload_unregister(priv->fwl);
 }
 
-- 
2.43.0

[PATCH net 08/10] net: pse-pd: pd692x0: defer probe while a manager supply is unresolved

[Carlo Szelinsky]

pd692x0_req_managers_pw_budget() uses each manager regulator's supply
without a NULL check. The supply is NULL while the vmain regulator is
described but not registered yet, because regulator_register() does not
wait for its supply. Probe then crashes.

Return -EPROBE_DEFER in that case. On net, each retry also leaks the
notification kfifo and the PI array until vmain appears; the fix for
that is in the PSE core fixes for net-next.

Fixes: 359754013e6a ("net: pse-pd: pd692x0: Add support for PSE PI priority feature")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pd692x0.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/pse-pd/pd692x0.c b/drivers/net/pse-pd/pd692x0.c
index 34f0dfbccd06..568789bd912f 100644
--- a/drivers/net/pse-pd/pd692x0.c
+++ b/drivers/net/pse-pd/pd692x0.c
@@ -1056,6 +1056,15 @@ pd692x0_req_managers_pw_budget(struct pd692x0_priv *priv)
 		struct regulator *supply = priv->manager_reg[i]->supply;
 		int pw_budget;
 
+		/* The regulator core registers the manager regulator even
+		 * when its vmain supply cannot be resolved yet and retries
+		 * later, leaving ->supply NULL until then.
+		 */
+		if (!supply)
+			return dev_err_probe(&priv->client->dev, -EPROBE_DEFER,
+					     "manager %d: vmain supply not resolved\n",
+					     i);
+
 		pw_budget = regulator_get_unclaimed_power_budget(supply);
 		if (!pw_budget)
 			/* Do nothing if no power budget */
-- 
2.43.0

[PATCH net 09/10] net: pse-pd: si3474: fix use-after-free in ancillary client error path

[Carlo Szelinsky]

If devm_add_action_or_reset() fails, it has already unregistered the
ancillary I2C client, and the error message then reads that client's
device. It can only fail with -ENOMEM, which is already reported, so
drop the message.

Fixes: a2317231df4b ("net: pse-pd: Add Si3474 PSE controller driver")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/si3474.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/pse-pd/si3474.c b/drivers/net/pse-pd/si3474.c
index 1845b9c51cf7..2f4f241a48af 100644
--- a/drivers/net/pse-pd/si3474.c
+++ b/drivers/net/pse-pd/si3474.c
@@ -516,10 +516,8 @@ static int si3474_i2c_probe(struct i2c_client *client)
 		return PTR_ERR(priv->client[1]);
 
 	ret = devm_add_action_or_reset(dev, si3474_ancillary_i2c_remove, priv->client[1]);
-	if (ret < 0) {
-		dev_err(&priv->client[1]->dev, "Cannot register remove callback\n");
+	if (ret)
 		return ret;
-	}
 
 	ret = i2c_smbus_read_byte_data(priv->client[1], VENDOR_IC_ID_REG);
 	if (ret < 0) {
-- 
2.43.0

[PATCH net 10/10] net: pse-pd: reject a power limit whose current does not fit an int

[Carlo Szelinsky]

pse_ethtool_set_pw_limit() turns the power limit into a current as
mW * 10^9 / uV and stores it in an int. For the largest allowed limit,
99900 mW, that overflows below about 46.5 mV. Only 0 V is rejected, and
si3474 reports the measured voltage of each port. A negative result then
hits the BUG_ON() in regulator_check_current_limit().

pse_pi_get_current_limit() does the same conversion and can return a
wrapped value that looks like an error code.

Do the conversion in one helper that returns -ERANGE when the result
does not fit an int.

Fixes: 4a83abcef5f4 ("net: pse-pd: Add new power limit get and set c33 features")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 0d1e5d5039f8..3e333bef4e48 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -741,12 +741,19 @@ static int pse_pi_get_voltage(struct regulator_dev *rdev)
 	return ret;
 }
 
+/* uA = mW * 1000000000 / uV, which does not fit an int for a low uV */
+static int pse_mw_to_ua(unsigned int mW, int uV)
+{
+	u64 uA = DIV_ROUND_CLOSEST_ULL((u64)mW * 1000000000ull, uV);
+
+	return uA > INT_MAX ? -ERANGE : uA;
+}
+
 static int pse_pi_get_current_limit(struct regulator_dev *rdev)
 {
 	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
 	const struct pse_controller_ops *ops;
 	int id, uV, mW, ret;
-	s64 tmp_64;
 
 	ops = pcdev->ops;
 	id = rdev_get_id(rdev);
@@ -769,10 +776,7 @@ static int pse_pi_get_current_limit(struct regulator_dev *rdev)
 		goto out;
 	uV = ret;
 
-	tmp_64 = mW;
-	tmp_64 *= 1000000000ull;
-	/* uA = mW * 1000000000 / uV */
-	ret = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+	ret = pse_mw_to_ua(mW, uV);
 
 out:
 	mutex_unlock(&pcdev->lock);
@@ -1910,7 +1914,6 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
 {
 	int uV, uA, ret, previous_pw_allocated = 0;
 	bool updated;
-	s64 tmp_64;
 
 	if (pw_limit > MAX_PI_PW)
 		return -ERANGE;
@@ -1928,10 +1931,12 @@ int pse_ethtool_set_pw_limit(struct pse_control *psec,
 	}
 	uV = ret;
 
-	tmp_64 = pw_limit;
-	tmp_64 *= 1000000000ull;
-	/* uA = mW * 1000000000 / uV */
-	uA = DIV_ROUND_CLOSEST_ULL(tmp_64, uV);
+	uA = pse_mw_to_ua(pw_limit, uV);
+	if (uA < 0) {
+		NL_SET_ERR_MSG(extack,
+			       "PSE voltage read is too low to set the limit");
+		return uA;
+	}
 
 	/* Update power budget only in software power control case and
 	 * if a Power Device is powered.
-- 
2.43.0