[PATCH] drivers: video: backlight: Fix NULL Pointer Dereference in backlight_device_register()

In the function "wled_probe", the "wled->name" is dynamically allocated
(wled_probe -> wled_configure -> devm_kasprintf), which is possible
to be null.

In the call trace: wled_probe -> devm_backlight_device_register
-> backlight_device_register, this "name" variable is directly
dereferenced without checking. We add a null-check statement.

Fixes: f86b77583d88 ("backlight: pm8941: Convert to using %pOFn instead of device_node.name")
Signed-off-by: Haoyu Li <redacted>
Cc: stable@vger.kernel.org
---
 drivers/video/backlight/backlight.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index f699e5827ccb..b21670bd86de 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c

@@ -414,6 +414,8 @@ struct backlight_device *backlight_device_register(const char *name,
 	struct backlight_device *new_bd;
 	int rc;
 
+	if (!name)
+		return ERR_PTR(-EINVAL);
 	pr_debug("backlight_device_register: name=%s\n", name);
 
 	new_bd = kzalloc(sizeof(struct backlight_device), GFP_KERNEL);

-- 
2.34.1

On Thu, 30 Jan 2025, Haoyu Li [off-list ref] wrote:

In the function "wled_probe", the "wled->name" is dynamically allocated
(wled_probe -> wled_configure -> devm_kasprintf), which is possible
to be null.

In the call trace: wled_probe -> devm_backlight_device_register
-> backlight_device_register, this "name" variable is directly
dereferenced without checking. We add a null-check statement.

Fixes: f86b77583d88 ("backlight: pm8941: Convert to using %pOFn instead of device_node.name")
Signed-off-by: Haoyu Li <redacted>
Cc: stable@vger.kernel.org

IMO whoever allocates should be responsible for checking NULL instead of
passing NULL around and expecting everyone check their input for NULL.

BR,
Jani.

-- 
Jani Nikula, Intel

On Mon, Feb 03, 2025 at 03:21:23PM +0200, Jani Nikula wrote:

On Thu, 30 Jan 2025, Haoyu Li [off-list ref] wrote:

quoted

In the function "wled_probe", the "wled->name" is dynamically allocated
(wled_probe -> wled_configure -> devm_kasprintf), which is possible
to be null.

In the call trace: wled_probe -> devm_backlight_device_register
-> backlight_device_register, this "name" variable is directly
dereferenced without checking. We add a null-check statement.

Fixes: f86b77583d88 ("backlight: pm8941: Convert to using %pOFn instead of device_node.name")
Signed-off-by: Haoyu Li <redacted>
Cc: stable@vger.kernel.org

IMO whoever allocates should be responsible for checking NULL instead of
passing NULL around and expecting everyone check their input for NULL.

Agreed. This should be fixed in at callsites.


Daniel.

As per Jani and Daniel's feedback, I have updated the patch so that
the `wled->name` null check now occurs in the `wled_configure`
function, right after the `devm_kasprintf` callsite. This should
resolve the issue.
The updated patch is as follows:

In the function "wled_probe", the "wled->name" is dynamically allocated
(wled_probe -> wled_configure -> devm_kasprintf), and it is possible
for it to be NULL.

To avoid dereferencing a NULL pointer (wled_probe ->
devm_backlight_device_register -> backlight_device_register),
we add a null-check after the allocation rather than in
backlight_device_register.

Fixes: f86b77583d88 ("backlight: pm8941: Convert to using %pOFn instead of device_node.name")
Signed-off-by: Haoyu Li <redacted>
Cc: stable@vger.kernel.org
---
 drivers/video/backlight/qcom-wled.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/video/backlight/qcom-wled.c b/drivers/video/backlight/qcom-wled.c
index 9afe701b2a1b..3dacfef821ca 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c

@@ -1409,6 +1409,11 @@ static int wled_configure(struct wled *wled)
 	if (rc)
 		wled->name = devm_kasprintf(dev, GFP_KERNEL, "%pOFn", dev->of_node);
 
+	if (!wled->name) {
+		dev_err(wled->dev, "Fail to initialize wled name\n");
+		return -EINVAL;
+	}
+
 	switch (wled->version) {
 	case 3:
 		u32_opts = wled3_opts;

-- 
2.34.1

Hi Haoyu

On Wed, Feb 19, 2025 at 08:29:50PM +0800, Haoyu Li wrote:

As per Jani and Daniel's feedback, I have updated the patch so that
the `wled->name` null check now occurs in the `wled_configure`
function, right after the `devm_kasprintf` callsite. This should
resolve the issue.

I'm afraid this patch got swamped in my mailbox and I missed it.

Worse, we've just been discussing and reviewing a patch for the same
issue from another developer:
https://lore.kernel.org/all/20250401091647.22784-1-bsdhenrymartin@gmail.com/

So, I just wanted to acknowlege the mistake. Sorry.


Daniel.