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

From: Joe Perches
Date: Tue Oct 29 2019 - 14:36:09 EST


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.
[]
> diff --git 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)

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