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

On Fri, Jan 3, 2020 at 2:09 PM Bartlomiej Zolnierkiewicz
[off-list ref] wrote:

On 10/29/19 8:02 PM, Eric W. Biederman wrote:

quoted

The goal is to avoid memory that has values of the previous users of
that memory region from leaking to userspace.  Which depending on who
the previous user of that memory region is could tell userspace
information about what the kernel is doing that it should not be allowed
to find out.

I tried to trace through where "info" and thus presumably "info->fix" is
coming from and only made it as far as  register_framebuffer.  Given

"info" (and thus "info->fix") comes from framebuffer_alloc() (which is
called by fbdev device drivers prior to registering "info" with
register_framebuffer()). framebuffer_alloc() does kzalloc() on "info".

Therefore shouldn't memcpy() (as suggested by Jeo Perches) be enough?

Is it guaranteed that all drivers call framebuffer_alloc() rather than
open-coding it somewhere?

Here is a list of all files that call register_framebuffer() without first
calling framebuffer_alloc:

$ git grep -wl register_framebuffer | xargs grep -L framebuffer_alloc
Documentation/fb/framebuffer.rst
drivers/media/pci/ivtv/ivtvfb.c
drivers/media/platform/vivid/vivid-osd.c
drivers/video/fbdev/68328fb.c
drivers/video/fbdev/acornfb.c
drivers/video/fbdev/amba-clcd.c
drivers/video/fbdev/atafb.c
drivers/video/fbdev/au1100fb.c
drivers/video/fbdev/controlfb.c
drivers/video/fbdev/core/fbmem.c
drivers/video/fbdev/cyber2000fb.c
drivers/video/fbdev/fsl-diu-fb.c
drivers/video/fbdev/g364fb.c
drivers/video/fbdev/goldfishfb.c
drivers/video/fbdev/hpfb.c
drivers/video/fbdev/macfb.c
drivers/video/fbdev/matrox/matroxfb_base.c
drivers/video/fbdev/matrox/matroxfb_crtc2.c
drivers/video/fbdev/maxinefb.c
drivers/video/fbdev/ocfb.c
drivers/video/fbdev/pxafb.c
drivers/video/fbdev/sa1100fb.c
drivers/video/fbdev/stifb.c
drivers/video/fbdev/valkyriefb.c
drivers/video/fbdev/vermilion/vermilion.c
drivers/video/fbdev/vt8500lcdfb.c
drivers/video/fbdev/wm8505fb.c
drivers/video/fbdev/xilinxfb.c

It's possible (even likely, the ones I looked at are fine) that they
all correctly
zero out the fb_info structure first, but it seems hard to guarantee, so
Eric's suggestion would possibly still be the safer choice.

      Arnd