Skip to contentSkip to conversation

fbdev: fix possible NULL pointer derefernce

PATCH In review Daeseok Youn 2 participants 4 messages Apr 16, 2014 – Apr 18, 2014

Subsystems (2)
Framebuffer layer Framebuffer core
lore
Share & tools
lore

Daeseok Youn

author
#0 lore raw

PATCH 1/1 fbdev: fix possible NULL pointer derefernce

The spec->modedb can be NULL by fb_create_modedb().

And also smatch says:
drivers/video/fbdev/core/fbmon.c:975 fb_edid_to_monspecs() error:
potential null dereference 'specs->modedb'. (fb_create_modedb returns null)

Signed-off-by Daeseok Youn <[email protected]>
---
 drivers/video/fbdev/core/fbmon.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
drivers/video/fbdev/core/fbmon.c +3 -0
index c204ebe..db274ca 100644
@@ -966,6 +966,9 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 
 	specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
 
+	if (!specs->modedb)
+		return;
+
 	/*
 	 * Workaround for buggy EDIDs that sets that the first
 	 * detailed timing is preferred but has not detailed
Signature · 1 line
1.7.4.4
#1 lore raw
Quoted 18 lines · Daeseok Youn

The spec->modedb can be NULL by fb_create_modedb().

And also smatch says:
drivers/video/fbdev/core/fbmon.c:975 fb_edid_to_monspecs() error:
potential null dereference 'specs->modedb'. (fb_create_modedb returns null)

Signed-off-by Daeseok Youn <[email protected]>

---
drivers/video/fbdev/core/fbmon.c | 3 +++

1 files changed, 3 insertions(+), 0 deletions(-)
drivers/video/fbdev/core/fbmon.c +0 -0
index c204ebe..db274ca 100644

drivers/video/fbdev/core/fbmon.c void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs) · line 971

Quoted 6 lines
drivers/video/fbdev/core/fbmon.c +3 -0
@@ -966,6 +966,9 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 
 	specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
 
+	if (!specs->modedb)
+		return;
+

we need to return an error and trace it

Best Regards,
J.

Quoted 6 lines
drivers/video/fbdev/core/fbmon.c +0 -0
 	/*
 	 * Workaround for buggy EDIDs that sets that the first
 	 * detailed timing is preferred but has not detailed
Signature · 1 line
1.7.4.4
author
#2 lore raw

Hello,

2014-04-16 21:38 GMT+09:00 Jean-Christophe PLAGNIOL-VILLARD <[email protected]>:

Quoted 22 lines · Daeseok Youn
Quoted 20 lines

The spec->modedb can be NULL by fb_create_modedb().

And also smatch says:
drivers/video/fbdev/core/fbmon.c:975 fb_edid_to_monspecs() error:
potential null dereference 'specs->modedb'. (fb_create_modedb returns null)

Signed-off-by Daeseok Youn <[email protected]>

---
drivers/video/fbdev/core/fbmon.c | 3 +++

1 files changed, 3 insertions(+), 0 deletions(-)
drivers/video/fbdev/core/fbmon.c +0 -0
index c204ebe..db274ca 100644
@@ -966,6 +966,9 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
Quoted 7 lines

drivers/video/fbdev/core/fbmon.c void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs) · line 971

Quoted 5 lines
drivers/video/fbdev/core/fbmon.c +3 -0
      specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
 
+     if (!specs->modedb)
+             return;
+

we need to return an error and trace it

Yes, you're right. I will change return type from void to int and add to handle an error when this function(fb_edid_to_monspecs) is called.

I will send this patch as your comment.

Thanks for review.

Daeseok Youn.

Quoted 10 lines

Best Regards,
J.

Quoted 6 lines
drivers/video/fbdev/core/fbmon.c +0 -1
      /*
       * Workaround for buggy EDIDs that sets that the first
       * detailed timing is preferred but has not detailed
--

1.7.4.4

author
#3 lore raw

Hello,

2014-04-17 9:00 GMT+09:00 DaeSeok Youn <[email protected]>:

Quoted 28 lines

Hello,

2014-04-16 21:38 GMT+09:00 Jean-Christophe PLAGNIOL-VILLARD <[email protected]>:

Quoted 24 lines · Daeseok Youn
Quoted 22 lines

The spec->modedb can be NULL by fb_create_modedb().

And also smatch says:
drivers/video/fbdev/core/fbmon.c:975 fb_edid_to_monspecs() error:
potential null dereference 'specs->modedb'. (fb_create_modedb returns null)

Signed-off-by Daeseok Youn <[email protected]>

---
drivers/video/fbdev/core/fbmon.c | 3 +++

1 files changed, 3 insertions(+), 0 deletions(-)
drivers/video/fbdev/core/fbmon.c +0 -0
index c204ebe..db274ca 100644
@@ -966,6 +966,9 @@ void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs)
 
      specs->modedb = fb_create_modedb(edid, &specs->modedb_len);
Quoted 8 lines
Quoted 5 lines

drivers/video/fbdev/core/fbmon.c void fb_edid_to_monspecs(unsigned char *edid, struct fb_monspecs *specs) · line 971

Quoted 3 lines
drivers/video/fbdev/core/fbmon.c +3 -0
+     if (!specs->modedb)
+             return;
+

we need to return an error and trace it

Yes, you're right. I will change return type from void to int and add to handle an error when this function(fb_edid_to_monspecs) is called.

I try to look at the fbdev code related with fb_edid_to_monspecs(). And the result, I think it doesn't need to return an error and trace it. My patch is also useless because modedb_len is zero when fb_create_modedb() returns NULL so It cannot be dereferenced.

And callers of this function seem to need a log but I think they don't need to handling an error. Some functions are handling error with checking NULL of modedb variable,
they are just printing an error message.

If I am wrong or you have reasons for handling an error on that function, please let me know.

Regards,
Daeseok Youn.

Quoted 15 lines

I will send this patch as your comment.

Thanks for review.

Daeseok Youn.

Quoted 10 lines

Best Regards,
J.

Quoted 6 lines
drivers/video/fbdev/core/fbmon.c +0 -1
      /*
       * Workaround for buggy EDIDs that sets that the first
       * detailed timing is preferred but has not detailed
--

1.7.4.4