linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] OMAP: DSS2: OMAPFB: fix potential GPF
@ 2021-06-25 22:33 Pavel Skripkin
  2021-06-26 23:14 ` Aaro Koskinen
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Skripkin @ 2021-06-25 22:33 UTC (permalink / raw)
  To: gustavoars, sam, tomi.valkeinen
  Cc: linux-omap, linux-fbdev, dri-devel, linux-kernel,
	linux-kernel-mentees, Pavel Skripkin

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

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;
 }
 
-- 
2.32.0


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] OMAP: DSS2: OMAPFB: fix potential GPF
  2021-06-25 22:33 [PATCH] OMAP: DSS2: OMAPFB: fix potential GPF Pavel Skripkin
@ 2021-06-26 23:14 ` Aaro Koskinen
  2021-06-27  8:48   ` Pavel Skripkin
  0 siblings, 1 reply; 3+ messages in thread
From: Aaro Koskinen @ 2021-06-26 23:14 UTC (permalink / raw)
  To: Pavel Skripkin
  Cc: gustavoars, sam, tomi.valkeinen, linux-omap, linux-fbdev,
	dri-devel, linux-kernel, linux-kernel-mentees

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] OMAP: DSS2: OMAPFB: fix potential GPF
  2021-06-26 23:14 ` Aaro Koskinen
@ 2021-06-27  8:48   ` Pavel Skripkin
  0 siblings, 0 replies; 3+ messages in thread
From: Pavel Skripkin @ 2021-06-27  8:48 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: gustavoars, sam, tomi.valkeinen, linux-omap, linux-fbdev,
	dri-devel, linux-kernel, linux-kernel-mentees

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-27  8:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-25 22:33 [PATCH] OMAP: DSS2: OMAPFB: fix potential GPF Pavel Skripkin
2021-06-26 23:14 ` Aaro Koskinen
2021-06-27  8:48   ` Pavel Skripkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).