| Message ID | 20210625223323.13930-1-paskripkin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | OMAP: DSS2: OMAPFB: fix potential GPF | expand |
Hi, On Sat, Jun 26, 2021 at 01:33:23AM +0300, Pavel Skripkin wrote: > In case of allocation failures, all code paths was jumping > to this code: > > err: > kfree(fbi); > kfree(var); > kfree(fbops); > > return r; > > Since all 3 pointers placed on stack and don't initialized, they > will be filled with some random values, which leads to > deferencing random pointers in kfree(). Fix it by rewriting > error handling path. They are initialized before the first goto: [...] fbi = NULL; var = NULL; fbops = NULL; fbi = kzalloc(sizeof(*fbi), GFP_KERNEL); if (fbi == NULL) { r = -ENOMEM; goto err; } [...] A.
On Sun, 27 Jun 2021 02:14:23 +0300 Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > Hi, > > On Sat, Jun 26, 2021 at 01:33:23AM +0300, Pavel Skripkin wrote: > > In case of allocation failures, all code paths was jumping > > to this code: > > > > err: > > kfree(fbi); > > kfree(var); > > kfree(fbops); > > > > return r; > > > > Since all 3 pointers placed on stack and don't initialized, they > > will be filled with some random values, which leads to > > deferencing random pointers in kfree(). Fix it by rewriting > > error handling path. > > They are initialized before the first goto: > > [...] > fbi = NULL; > var = NULL; > fbops = NULL; > > fbi = kzalloc(sizeof(*fbi), GFP_KERNEL); > if (fbi == NULL) { > r = -ENOMEM; > goto err; > } > [...] > Hi! Im sorry for this, I should not stay to late night reviewing the code next time :( With regards, Pavel Skripkin
diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c index a3decc7fadde..6a302138ebeb 100644 --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c @@ -2025,21 +2025,19 @@ static int omapfb_mode_to_timings(const char *mode_str, fbops = NULL; fbi = kzalloc(sizeof(*fbi), GFP_KERNEL); - if (fbi == NULL) { - r = -ENOMEM; - goto err; - } + if (fbi == NULL) + return -ENOMEM; var = kzalloc(sizeof(*var), GFP_KERNEL); if (var == NULL) { r = -ENOMEM; - goto err; + goto err_var; } fbops = kzalloc(sizeof(*fbops), GFP_KERNEL); if (fbops == NULL) { r = -ENOMEM; - goto err; + goto err_fbops; } fbi->fbops = fbops; @@ -2047,7 +2045,7 @@ static int omapfb_mode_to_timings(const char *mode_str, r = fb_find_mode(var, fbi, mode_str, NULL, 0, NULL, 24); if (r == 0) { r = -EINVAL; - goto err; + goto err_find; } if (display->driver->get_timings) { @@ -2088,11 +2086,12 @@ static int omapfb_mode_to_timings(const char *mode_str, r = 0; -err: - kfree(fbi); - kfree(var); +err_find: kfree(fbops); - +err_fbops: + kfree(var); +err_var: + kfree(fbi); return r; }
In case of allocation failures, all code paths was jumping to this code: err: kfree(fbi); kfree(var); kfree(fbops); return r; Since all 3 pointers placed on stack and don't initialized, they will be filled with some random values, which leads to deferencing random pointers in kfree(). Fix it by rewriting error handling path. Fixes: 897044e99e43 ("OMAP: DSS2: OMAPFB: Reduce stack usage") Signed-off-by: Pavel Skripkin <paskripkin@gmail.com> --- .../video/fbdev/omap2/omapfb/omapfb-main.c | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-)