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

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

[Haoyu Li]

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 <lihaoyu499@gmail.com>
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

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

[Jani Nikula]

On Thu, 30 Jan 2025, Haoyu Li <lihaoyu499@gmail.com> 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 <lihaoyu499@gmail.com>
> 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.


> ---
>  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);

-- 
Jani Nikula, Intel

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

[Daniel Thompson]

On Mon, Feb 03, 2025 at 03:21:23PM +0200, Jani Nikula wrote:
> On Thu, 30 Jan 2025, Haoyu Li <lihaoyu499@gmail.com> 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 <lihaoyu499@gmail.com>
> > 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.

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

[Haoyu Li]

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 <lihaoyu499@gmail.com>
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

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

[Daniel Thompson]

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.