[PATCH] fbdev: potential information leak in do_fb_ioctl()

Subsystems: framebuffer core, framebuffer layer, the rest · Maintainers: Simona Vetter, Helge Deller, Linus Torvalds

STALE2554d

2 messages, 2 authors, 2019-10-29 · open the first message on its own page

[PATCH] fbdev: potential information leak in do_fb_ioctl()

From: Dan Carpenter <hidden>
Date: 2019-10-29 18:25:15

The "fix" struct has a 2 byte hole after ->ywrapstep and the
"fix = info->fix;" assignment doesn't necessarily clear it.  It depends
on the compiler.

Fixes: 1f5e31d7e55a ("fbmem: don't call copy_from/to_user() with mutex held")
Signed-off-by: Dan Carpenter <redacted>
---
I have 13 more similar places to patch...  I'm not totally sure I
understand all the issues involved.

 drivers/video/fbdev/core/fbmem.c | 1 +
 1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 6f6fc785b545..b4ce6a28aed9 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1109,6 +1109,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 			ret = -EFAULT;
 		break;
 	case FBIOGET_FSCREENINFO:
+		memset(&fix, 0, sizeof(fix));
 		lock_fb_info(info);
 		fix = info->fix;
 		if (info->flags & FBINFO_HIDE_SMEM_START)
-- 
2.20.1

Re: [PATCH] fbdev: potential information leak in do_fb_ioctl()

From: Joe Perches <joe@perches.com>
Date: 2019-10-29 18:36:09

On Tue, 2019-10-29 at 21:23 +0300, Dan Carpenter wrote:
The "fix" struct has a 2 byte hole after ->ywrapstep and the
"fix = info->fix;" assignment doesn't necessarily clear it.  It depends
on the compiler.
[]
quoted hunk
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
[]
quoted hunk
@@ -1109,6 +1109,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 			ret = -EFAULT;
 		break;
 	case FBIOGET_FSCREENINFO:
+		memset(&fix, 0, sizeof(fix));
 		lock_fb_info(info);
 		fix = info->fix;
 		if (info->flags & FBINFO_HIDE_SMEM_START)
Perhaps better to change the struct copy to a memcpy
---
 drivers/video/fbdev/core/fbmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index e6a1c80..364699 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1110,7 +1110,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
 		break;
 	case FBIOGET_FSCREENINFO:
 		lock_fb_info(info);
-		fix = info->fix;
+		memcpy(&fix, &info->fix, sizeof(fix));
 		if (info->flags & FBINFO_HIDE_SMEM_START)
 			fix.smem_start = 0;
 		unlock_fb_info(info);
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help