[PATCH 0/6] backlight: Fix Critical and High issues reported by Sashiko

[PATCH 0/6] backlight: Fix Critical and High issues reported by Sashiko

[Lee Jones]

These issues were reported by Sashiko.

This series resolves the open Critical and High severity bug reports
identified by the Sashiko AI analysis tool (https://sashiko.dev) across
the Backlight subsystem:

  1. lm3630a_bl: Prevent a NULL pointer dereference in lm3630a_pwm_ctrl()
     when PWM mode is enabled by requesting the PWM device before
     registering the backlight device.
  2. aat2870_bl: Prevent a NULL pointer dereference on platform_get_drvdata()
     during devm_backlight_device_register() by populating driver data and
     scaling the initial brightness beforehand.
  3. qcom-wled: Cancel wled->ovp_work on probe failure via
     devm_delayed_work_autocancel() so a scheduled OVP work item cannot
     fire after devm_kzalloc() frees struct wled.
  4. qcom-wled: Manage wled->lock via devm_mutex_init() and remove
     wled_remove() so devres unwinds the backlight device, threaded IRQs,
     delayed work, and mutex in strict reverse registration order without
     calling disable_irq() on unconfigured negative or zero IRQ numbers.
  5. qcom-wled: Guard against calling a NULL
     wled->wled_auto_detection_required() callback on WLED3 hardware when
     an OVP fault interrupt fires.
  6. qcom-wled: Avoid an AB-BA deadlock between wled->lock and the
     threaded OVP IRQ handler by using disable_irq_nosync() in
     wled_module_enable() and verifying under wled->lock that the module
     remains enabled before running wled_auto_string_detection().

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Bjorn Andersson <andersson@kernel.org>
Cc: Daniel Jeong <gshark.jeong@gmail.com>
Cc: Daniel Thompson <danielt@kernel.org>
Cc: David Heidelberg <david@ixit.cz>
Cc: dri-devel@lists.freedesktop.org
Cc: Helge Deller <deller@gmx.de>
Cc: Jingoo Han <jingoohan1@gmail.com>
Cc: Jin Park <jinyoungp@nvidia.com>
Cc: Kiran Gunda <quic_kgunda@quicinc.com>
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Cc: LDD MLP <ldd-mlp@list.ti.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: Samuel Ortiz <sameo@linux.intel.com>
Cc: Subbaraman Narayanamurthy <quic_subbaram@quicinc.com>

Lee Jones (6):
  backlight: lm3630a: Request PWM device before registering backlight
  backlight: aat2870: Initialise driver data before registering
    backlight
  backlight: qcom-wled: Cancel delayed work on probe failure
  backlight: qcom-wled: Manage teardown via devres and remove
    wled_remove()
  backlight: qcom-wled: Check for auto-detection callback in
    wled_ovp_irq_handler()
  backlight: qcom-wled: Avoid AB-BA deadlock in wled_module_enable()

 drivers/video/backlight/aat2870_bl.c | 43 +++++++++++++------------
 drivers/video/backlight/lm3630a_bl.c | 12 +++----
 drivers/video/backlight/qcom-wled.c  | 47 +++++++++++++++-------------
 3 files changed, 52 insertions(+), 50 deletions(-)

-- 
2.56.0.385.gac671c819a-goog

[PATCH 1/6] backlight: lm3630a: Request PWM device before registering backlight

[Lee Jones]

lm3630a_probe() currently registers the backlight devices via
lm3630a_backlight_register() before acquiring and initialising the PWM
device via devm_pwm_get() and pwm_init_state().

This creates two issues when PWM control is enabled:

1. Exposing the backlight sysfs attributes before pchip->pwmd is
   initialised leaves a window during probe where a userspace brightness
   update calls lm3630a_pwm_ctrl() with pchip->pwmd still NULL (or set
   to an ERR_PTR() if devm_pwm_get() fails while devres tears down the
   backlight device).

2. Because devres releases resources in reverse registration order on
   driver unbind, the PWM device is released before the backlight
   devices are unregistered, allowing a sysfs brightness write during
   unbind to invoke lm3630a_pwm_ctrl() on a freed PWM device.

Acquire and initialise the PWM device before registering the backlight
devices so that pchip->pwmd is valid throughout the lifetime of the
backlight registration.

Fixes: 28e64a68a2ef ("backlight: lm3630: apply chip revision")
Closes: https://sashiko.dev/#/bug/linux-ba01ed63-0b5a-4b3d-94c5-e6989857d77c
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/video/backlight/lm3630a_bl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/lm3630a_bl.c b/drivers/video/backlight/lm3630a_bl.c
index 8ef8163c7fef..9061355ef8ed 100644
--- a/drivers/video/backlight/lm3630a_bl.c
+++ b/drivers/video/backlight/lm3630a_bl.c
@@ -549,12 +549,6 @@ static int lm3630a_probe(struct i2c_client *client)
 		dev_err(&client->dev, "fail : init chip\n");
 		return rval;
 	}
-	/* backlight register */
-	rval = lm3630a_backlight_register(pchip);
-	if (rval < 0) {
-		dev_err(&client->dev, "fail : backlight register.\n");
-		return rval;
-	}
 	/* pwm */
 	if (pdata->pwm_ctrl != LM3630A_PWM_DISABLE) {
 		pchip->pwmd = devm_pwm_get(pchip->dev, "lm3630a-pwm");
@@ -564,6 +558,12 @@ static int lm3630a_probe(struct i2c_client *client)
 
 		pwm_init_state(pchip->pwmd, &pchip->pwmd_state);
 	}
+	/* backlight register */
+	rval = lm3630a_backlight_register(pchip);
+	if (rval < 0) {
+		dev_err(&client->dev, "fail : backlight register.\n");
+		return rval;
+	}
 
 	/* interrupt enable  : irq 0 is not allowed */
 	pchip->irq = client->irq;
-- 
2.56.0.385.gac671c819a-goog

[PATCH 2/6] backlight: aat2870: Initialise driver data before registering backlight

[Lee Jones]

aat2870_bl_probe() currently registers the backlight device via
devm_backlight_device_register() before initialising its private driver
data (aat2870_bl->pdev, channels, and max_current) and backlight
properties (max_brightness, power, and brightness).

Because devm_backlight_device_register() immediately exposes the
backlight sysfs interface to userspace, a concurrent sysfs write to
attributes such as brightness invokes aat2870_bl_update_status() while
aat2870_bl->pdev is still NULL, causing a NULL pointer dereference.  In
addition, aat2870_brightness() reads the backlight_device pointer from
aat2870_bl->bd (which is only assigned after registration returns)
rather than using the backlight_device pointer passed into
aat2870_bl_update_status().

Populate the driver data and backlight_properties structure prior to
calling devm_backlight_device_register(), and pass the backlight_device
pointer directly to aat2870_brightness().

Fixes: 4b0d711be39f ("backlight: Add AAT2870 backlight driver")
Closes: https://sashiko.dev/#/bug/linux-9a61532b-5d7a-467d-b3d1-d6420978c8f3
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
 drivers/video/backlight/aat2870_bl.c | 43 ++++++++++++++--------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/video/backlight/aat2870_bl.c b/drivers/video/backlight/aat2870_bl.c
index 8b790df1e842..53eaa3e1e7a3 100644
--- a/drivers/video/backlight/aat2870_bl.c
+++ b/drivers/video/backlight/aat2870_bl.c
@@ -24,10 +24,10 @@ struct aat2870_bl_driver_data {
 	int brightness; /* current brightness */
 };
 
-static inline int aat2870_brightness(struct aat2870_bl_driver_data *aat2870_bl,
+static inline int aat2870_brightness(struct backlight_device *bd,
 				     int brightness)
 {
-	struct backlight_device *bd = aat2870_bl->bd;
+	struct aat2870_bl_driver_data *aat2870_bl = bl_get_data(bd);
 	int val;
 
 	val = brightness * (aat2870_bl->max_current - 1);
@@ -70,7 +70,7 @@ static int aat2870_bl_update_status(struct backlight_device *bd)
 		 bd->props.brightness, bd->props.power, bd->props.state);
 
 	ret = aat2870->write(aat2870, AAT2870_BLM,
-			     (u8)aat2870_brightness(aat2870_bl, brightness));
+			     (u8)aat2870_brightness(bd, brightness));
 	if (ret < 0)
 		return ret;
 
@@ -122,24 +122,9 @@ static int aat2870_bl_probe(struct platform_device *pdev)
 		goto out;
 	}
 
-	memset(&props, 0, sizeof(struct backlight_properties));
-
-	props.type = BACKLIGHT_RAW;
-	bd = devm_backlight_device_register(&pdev->dev, "aat2870-backlight",
-					&pdev->dev, aat2870_bl, &aat2870_bl_ops,
-					&props);
-	if (IS_ERR(bd)) {
-		dev_err(&pdev->dev,
-			"Failed allocate memory for backlight device\n");
-		ret = PTR_ERR(bd);
-		goto out;
-	}
-
 	aat2870_bl->pdev = pdev;
 	platform_set_drvdata(pdev, aat2870_bl);
 
-	aat2870_bl->bd = bd;
-
 	if (pdata->channels > 0)
 		aat2870_bl->channels = pdata->channels;
 	else
@@ -150,14 +135,28 @@ static int aat2870_bl_probe(struct platform_device *pdev)
 	else
 		aat2870_bl->max_current = AAT2870_CURRENT_27_9;
 
+	memset(&props, 0, sizeof(struct backlight_properties));
+
+	props.type = BACKLIGHT_RAW;
 	if (pdata->max_brightness > 0)
-		bd->props.max_brightness = pdata->max_brightness;
+		props.max_brightness = pdata->max_brightness;
 	else
-		bd->props.max_brightness = 255;
+		props.max_brightness = 255;
 
 	aat2870_bl->brightness = 0;
-	bd->props.power = BACKLIGHT_POWER_ON;
-	bd->props.brightness = bd->props.max_brightness;
+	props.power = BACKLIGHT_POWER_ON;
+	props.brightness = props.max_brightness;
+
+	bd = devm_backlight_device_register(&pdev->dev, "aat2870-backlight",
+					    &pdev->dev, aat2870_bl,
+					    &aat2870_bl_ops, &props);
+	if (IS_ERR(bd)) {
+		dev_err(&pdev->dev,
+			"Failed allocate memory for backlight device\n");
+		ret = PTR_ERR(bd);
+		goto out;
+	}
+	aat2870_bl->bd = bd;
 
 	ret = aat2870_bl_update_status(bd);
 	if (ret < 0) {
-- 
2.56.0.385.gac671c819a-goog

[PATCH 3/6] backlight: qcom-wled: Cancel delayed work on probe failure

[Lee Jones]

If a short interrupt fires after wled_configure_short_irq() has
registered the interrupt handler, wled_short_irq_handler() calls
wled_module_enable(), which schedules wled->ovp_work.

If wled_probe() subsequently fails during
devm_backlight_device_register(), devres frees the wled structure while
wled->ovp_work remains pending, leading to a use-after-free when
wled_ovp_work() executes.

Configure the OVP interrupt first, then initialise wled->ovp_work via
devm_delayed_work_autocancel() before requesting the short interrupt.
This ensures that on teardown devres frees the short interrupt first,
cancels any pending wled->ovp_work while wled->ovp_irq is still valid,
and finally frees the OVP interrupt and wled structure.

Fixes: 8663c188beea ("backlight: qcom-wled: Add auto string detection logic")
Closes: https://sashiko.dev/#/bug/linux-ca2e48a2-f511-44b8-a7d4-866f360bf107
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
Cc: David Heidelberg <david@ixit.cz>
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

 drivers/video/backlight/qcom-wled.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index ca350f320e17..f8f41e1d6ccf 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -3,6 +3,7 @@
  */
 
 #include <linux/delay.h>
+#include <linux/devm-helpers.h>
 #include <linux/interrupt.h>
 #include <linux/ktime.h>
 #include <linux/kernel.h>
@@ -1784,8 +1785,6 @@ static int wled_probe(struct platform_device *pdev)
 		break;
 	}
 
-	INIT_DELAYED_WORK(&wled->ovp_work, wled_ovp_work);
-
 	val = WLED_DEFAULT_BRIGHTNESS;
 	of_property_read_u32(pdev->dev.of_node, "default-brightness", &val);
 
@@ -1800,14 +1799,19 @@ static int wled_probe(struct platform_device *pdev)
 
 	wled->brightness = rc;
 
-	rc = wled_configure_short_irq(wled, pdev);
-	if (rc < 0)
-		return rc;
-
 	rc = wled_configure_ovp_irq(wled, pdev);
 	if (rc < 0)
 		return rc;
 
+	rc = devm_delayed_work_autocancel(&pdev->dev, &wled->ovp_work,
+					  wled_ovp_work);
+	if (rc)
+		return rc;
+
+	rc = wled_configure_short_irq(wled, pdev);
+	if (rc < 0)
+		return rc;
+
 	memset(&props, 0, sizeof(struct backlight_properties));
 	props.type = BACKLIGHT_RAW;
 	props.brightness = wled->brightness ?: val;
-- 
2.56.0.385.gac671c819a-goog

[PATCH 4/6] backlight: qcom-wled: Manage teardown via devres and remove wled_remove()

[Lee Jones]

wled_remove() currently runs before devres tears down the backlight
device and threaded interrupts, resulting in two teardown bugs:

1. mutex_destroy(&wled->lock) and cancel_delayed_work_sync(&wled->ovp_work)
   execute while the backlight device registered via
   devm_backlight_device_register() and the short interrupt requested
   via devm_request_threaded_irq() are still active.  A concurrent sysfs
   brightness update or short interrupt during unbind can acquire the
   destroyed mutex and re-schedule wled->ovp_work before devres frees
   the wled structure.

2. disable_irq(wled->short_irq) and disable_irq(wled->ovp_irq) are
   called unconditionally even when the optional "short" or "ovp"
   interrupts are absent from the Device Tree (-ENXIO) or unused (0 on
   WLED3), passing invalid IRQ numbers to disable_irq(), despite both
   interrupts already being managed by devm_request_threaded_irq().

Initialise wled->lock with devm_mutex_init() and remove wled_remove()
so that devres tears down the backlight device, short interrupt,
delayed work, OVP interrupt, mutex, and driver data in reverse
registration order.

Fixes: feeab87b3072 ("backlight: qcom-wled: Add support for short circuit handling")
Fixes: 8663c188beea ("backlight: qcom-wled: Add auto string detection logic")
Closes: https://sashiko.dev/#/bug/linux-51cd5e35-3cae-4e4c-986b-7261263507cd
Closes: https://sashiko.dev/#/bug/linux-44e5ed80-c35c-4786-8bf8-9ee955967d0b
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
Cc: David Heidelberg <david@ixit.cz>
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

 drivers/video/backlight/qcom-wled.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index f8f41e1d6ccf..8d4d7fe5e84d 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -1732,7 +1732,6 @@ static int wled_probe(struct platform_device *pdev)
 
 	wled->regmap = regmap;
 	wled->dev = &pdev->dev;
-	platform_set_drvdata(pdev, wled);
 
 	wled->version = (uintptr_t)of_device_get_match_data(&pdev->dev);
 	if (!wled->version) {
@@ -1740,7 +1739,10 @@ static int wled_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	mutex_init(&wled->lock);
+	rc = devm_mutex_init(&pdev->dev, &wled->lock);
+	if (rc)
+		return rc;
+
 	rc = wled_configure(wled);
 	if (rc)
 		return rc;
@@ -1822,16 +1824,6 @@ static int wled_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(bl);
 };
 
-static void wled_remove(struct platform_device *pdev)
-{
-	struct wled *wled = platform_get_drvdata(pdev);
-
-	mutex_destroy(&wled->lock);
-	cancel_delayed_work_sync(&wled->ovp_work);
-	disable_irq(wled->short_irq);
-	disable_irq(wled->ovp_irq);
-}
-
 static const struct of_device_id wled_match_table[] = {
 	{ .compatible = "qcom,pm8941-wled", .data = (void *)3 },
 	{ .compatible = "qcom,pmi8950-wled", .data = (void *)4 },
@@ -1846,7 +1838,6 @@ MODULE_DEVICE_TABLE(of, wled_match_table);
 
 static struct platform_driver wled_driver = {
 	.probe = wled_probe,
-	.remove = wled_remove,
 	.driver	= {
 		.name = "qcom,wled",
 		.of_match_table	= wled_match_table,
-- 
2.56.0.385.gac671c819a-goog

[PATCH 5/6] backlight: qcom-wled: Check for auto-detection callback in wled_ovp_irq_handler()

[Lee Jones]

The wled_auto_detection_required callback is only populated for WLED4
and WLED5 hardware, remaining NULL on WLED3 (where automatic string
detection is not supported).

However, wled_configure_ovp_irq() still registers wled_ovp_irq_handler()
on WLED3 when an "ovp" interrupt is present in the Device Tree.  If an
OVP fault interrupt fires on WLED3, wled_ovp_irq_handler()
unconditionally invokes wled->wled_auto_detection_required(), causing a
NULL pointer dereference.

Verify that wled->wled_auto_detection_required is non-NULL before
invoking it in wled_ovp_irq_handler().

Fixes: f16899a6a7d4 ("backlight: qcom-wled: Add callback functions")
Closes: https://sashiko.dev/#/bug/linux-d3eba287-2af9-43fb-851f-78ab3df0f35b
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
Cc: David Heidelberg <david@ixit.cz>
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

 drivers/video/backlight/qcom-wled.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 8d4d7fe5e84d..06300d403f8f 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -917,7 +917,8 @@ static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
 			int_sts, fault_sts);
 
 	if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
-		if (wled->wled_auto_detection_required(wled)) {
+		if (wled->wled_auto_detection_required &&
+		    wled->wled_auto_detection_required(wled)) {
 			mutex_lock(&wled->lock);
 			wled_auto_string_detection(wled);
 			mutex_unlock(&wled->lock);
-- 
2.56.0.385.gac671c819a-goog

[PATCH 6/6] backlight: qcom-wled: Avoid AB-BA deadlock in wled_module_enable()

[Lee Jones]

When disabling the WLED module (for example from wled_update_status() or
wled_short_irq_handler()), the caller holds wled->lock and invokes
wled_module_enable(wled, false), which calls disable_irq(wled->ovp_irq)
and blocks waiting for any executing OVP threaded interrupt handler to
complete.

If wled_ovp_irq_handler() is concurrently executing, it attempts to
acquire wled->lock before running wled_auto_string_detection(),
resulting in an AB-BA deadlock between wled->lock and IRQ completion.

Use disable_irq_nosync() instead of disable_irq() in
wled_module_enable() so that the OVP interrupt is masked without
synchronously waiting for the threaded handler while holding wled->lock.
Because wled_ovp_irq_handler() may already be waiting on wled->lock when
the module is disabled, verify under wled->lock that the module is still
enabled and not disabled by a short before running
wled_auto_string_detection().

Fixes: 8663c188beea ("backlight: qcom-wled: Add auto string detection logic")
Closes: https://sashiko.dev/#/bug/linux-3afa151c-7b8f-432c-8f85-8c43a83b964a
Cc: stable@vger.kernel.org
Signed-off-by: Lee Jones <lee@kernel.org>
---
Cc: David Heidelberg <david@ixit.cz>
Cc: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

 drivers/video/backlight/qcom-wled.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 06300d403f8f..6c0f631cc975 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -367,7 +367,7 @@ static int wled_module_enable(struct wled *wled, int val)
 			schedule_delayed_work(&wled->ovp_work, HZ / 100);
 		} else {
 			if (!cancel_delayed_work_sync(&wled->ovp_work))
-				disable_irq(wled->ovp_irq);
+				disable_irq_nosync(wled->ovp_irq);
 		}
 	}
 
@@ -919,8 +919,15 @@ static irqreturn_t wled_ovp_irq_handler(int irq, void *_wled)
 	if (fault_sts & WLED3_CTRL_REG_OVP_FAULT_BIT) {
 		if (wled->wled_auto_detection_required &&
 		    wled->wled_auto_detection_required(wled)) {
+			u32 val;
+
 			mutex_lock(&wled->lock);
-			wled_auto_string_detection(wled);
+			if (!wled->disabled_by_short) {
+				rc = regmap_read(wled->regmap, wled->ctrl_addr +
+						 WLED3_CTRL_REG_MOD_EN, &val);
+				if (!rc && (val & WLED3_CTRL_REG_MOD_EN_MASK))
+					wled_auto_string_detection(wled);
+			}
 			mutex_unlock(&wled->lock);
 		}
 	}
-- 
2.56.0.385.gac671c819a-goog