[PATCH] video: fbdev: atmel_lcdfb: fix list_first_entry() on possibly empty modelist in probe

[PATCH] video: fbdev: atmel_lcdfb: fix list_first_entry() on possibly empty modelist in probe

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

list_first_entry() does not return NULL on an empty list; it returns a
bogus pointer computed by container_of() from the list head.
atmel_lcdfb_probe() initializes info->modelist with INIT_LIST_HEAD()
and later takes its first entry with list_first_entry(), assuming
atmel_lcdfb_of_init() populated it. However, atmel_lcdfb_of_init()
drops the return value of fb_add_videomode() — unlike every other
fallible call in the same function — so if fb_add_videomode() fails
(its internal kmalloc can return -ENOMEM under memory pressure or
fault injection), the list stays empty while of_init() reports
success. The probe path then dereferences the bogus pointer in
fb_videomode_to_var(&info->var, &modelist->mode). Because the bogus
pointer lands inside the contiguous fb_info allocation, this may not
fault immediately; instead it silently reads adjacent memory as a
struct fb_videomode and programs garbage timing parameters into the
LCD controller. This was found by code audit; we do not have Atmel
hardware to reproduce it.

Fix this on two levels: check the return value of fb_add_videomode()
in atmel_lcdfb_of_init() so the failure is propagated (root cause),
and use list_first_entry_or_null() in atmel_lcdfb_probe() with a NULL
check that fails probe gracefully with -EINVAL (defense in depth),
consistent with the surrounding error handling which bails out to
stop_clk.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
 drivers/video/fbdev/atmel_lcdfb.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbdev/atmel_lcdfb.c b/drivers/video/fbdev/atmel_lcdfb.c
index 53f015699..6820a2400 100644
--- a/drivers/video/fbdev/atmel_lcdfb.c
+++ b/drivers/video/fbdev/atmel_lcdfb.c
@@ -1007,7 +1007,9 @@ static int atmel_lcdfb_of_init(struct atmel_lcdfb_info *sinfo)
 	if (ret < 0)
 		goto put_display_node;
 
-	fb_add_videomode(&fb_vm, &info->modelist);
+	ret = fb_add_videomode(&fb_vm, &info->modelist);
+	if (ret < 0)
+		goto put_display_node;
 
 put_display_node:
 	of_node_put(display_np);
@@ -1075,8 +1077,13 @@ static int atmel_lcdfb_probe(struct platform_device *pdev)
 	}
 	atmel_lcdfb_start_clock(sinfo);
 
-	modelist = list_first_entry(&info->modelist,
-			struct fb_modelist, list);
+	modelist = list_first_entry_or_null(&info->modelist,
+					    struct fb_modelist, list);
+	if (!modelist) {
+		dev_err(dev, "no video mode available\n");
+		ret = -EINVAL;
+		goto stop_clk;
+	}
 	fb_videomode_to_var(&info->var, &modelist->mode);
 
 	atmel_lcdfb_check_var(&info->var, info);
-- 
2.30.2

Re: [PATCH] video: fbdev: atmel_lcdfb: fix list_first_entry() on possibly empty modelist in probe

[Helge Deller]

On 10/23/26 09:56, Li Ximo wrote:
> From: Wang Haoran <haoranwangsec@gmail.com>
> 
> list_first_entry() does not return NULL on an empty list; it returns a
> bogus pointer computed by container_of() from the list head.
> atmel_lcdfb_probe() initializes info->modelist with INIT_LIST_HEAD()
> and later takes its first entry with list_first_entry(), assuming
> atmel_lcdfb_of_init() populated it. However, atmel_lcdfb_of_init()
> drops the return value of fb_add_videomode() — unlike every other
> fallible call in the same function — so if fb_add_videomode() fails
> (its internal kmalloc can return -ENOMEM under memory pressure or
> fault injection), the list stays empty while of_init() reports
> success. The probe path then dereferences the bogus pointer in
> fb_videomode_to_var(&info->var, &modelist->mode). Because the bogus
> pointer lands inside the contiguous fb_info allocation, this may not
> fault immediately; instead it silently reads adjacent memory as a
> struct fb_videomode and programs garbage timing parameters into the
> LCD controller. This was found by code audit; we do not have Atmel
> hardware to reproduce it.
> 
> Fix this on two levels: check the return value of fb_add_videomode()
> in atmel_lcdfb_of_init() so the failure is propagated (root cause),
> and use list_first_entry_or_null() in atmel_lcdfb_probe() with a NULL
> check that fails probe gracefully with -EINVAL (defense in depth),
> consistent with the surrounding error handling which bails out to
> stop_clk.
> 
> Reported-by: Wang Haoran <haoranwangsec@gmail.com>
> Reported-by: Li Ximo <lixicaicai@gmail.com>
> Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
> ---
>   drivers/video/fbdev/atmel_lcdfb.c | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
applied.

Thanks!
Helge