fbdev: sm501fb: Fix buffer errors in OF binding code

The code that gets the frame buffer mode from OF has 'use after free',
'buffer overrun' and memory leaks.

info->edid_data isn't free if the probe functions fail or if
pd->def_mode is set.

If both the CRT and PANEL are enabled info->edid_data is used after
being freed and is freed twice.

The string returned by of_get_property(np, "mode", &len) is just
written over either the static "640x480-16@60" or the module parameter
string without any regard for the length (which is most likely longer).

Use kstrump() for the OF mode and free everything before freeing 'info.

Fixes: 4295f9bf74a88 ("video, sm501: add OF binding to support SM501")
Signed-off-by: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/drivers/video/fbdev/sm501fb.c b/drivers/video/fbdev/sm501fb.c
index fee4b9f84592f7b728e1988ef43ced69786cdaae..ea5375ed4ea6f3da75e20d6b6a4a3a26d83076f8 100644 (file)
--- a/drivers/video/fbdev/sm501fb.c
+++ b/drivers/video/fbdev/sm501fb.c
@@ -96,6 +96,7 @@ struct sm501fb_info {
        void __iomem            *fbmem;         /* remapped framebuffer */
        size_t                   fbmem_len;     /* length of remapped region */
        u8 *edid_data;
+       char *fb_mode;
 };
 
 /* per-framebuffer private data */
@@ -1793,12 +1794,11 @@ static int sm501fb_init_fb(struct fb_info *fb, enum sm501_controller head,
                        fb->var.yres_virtual = fb->var.yres;
                } else {
                        if (info->edid_data) {
-                               ret = fb_find_mode(&fb->var, fb, fb_mode,
+                               ret = fb_find_mode(&fb->var, fb,
+                                       info->fb_mode ?: fb_mode,
                                        fb->monspecs.modedb,
                                        fb->monspecs.modedb_len,
                                        &sm501_default_mode, default_bpp);
-                               /* edid_data is no longer needed, free it */
-                               kfree(info->edid_data);
                        } else {
                                ret = fb_find_mode(&fb->var, fb,
                                           NULL, NULL, 0, NULL, 8);
@@ -1974,7 +1974,7 @@ static int sm501fb_probe(struct platform_device *pdev)
                        /* Get EDID */
                        cp = of_get_property(np, "mode", &len);
                        if (cp)
-                               strcpy(fb_mode, cp);
+                               info->fb_mode = kstrdup(cp, GFP_KERNEL);
                        prop = of_get_property(np, "edid", &len);
                        if (prop && len == EDID_LENGTH) {
                                info->edid_data = kmemdup(prop, EDID_LENGTH,
@@ -2031,6 +2031,12 @@ static int sm501fb_probe(struct platform_device *pdev)
                goto err_started_crt;
        }
 
+       /* These aren't needed any more */
+       kfree(info->edid_data);
+       kfree(info->fb_mode);
+       info->edid_data = NULL;
+       info->fb_mode = NULL;
+
        /* we registered, return ok */
        return 0;
 
@@ -2048,6 +2054,8 @@ err_probed_crt:
        framebuffer_release(info->fb[HEAD_CRT]);
 
 err_alloc:
+       kfree(info->edid_data);
+       kfree(info->fb_mode);
        kfree(info);
 
        return ret;