[Intel-gfx] [PATCH] drm/i915/fbdev: Hide smem_start from userspace

[Chris Wilson]

Quoting Ville Syrjälä (2019-11-14 10:53:11)
> On Wed, Nov 13, 2019 at 05:19:44PM +0000, Chris Wilson wrote:
> > Do not leak our internal kernel address for random userspace to abuse.
> > Daniel added the support to fbdev to filter out the physical addresses
> > being exposed by fbdev, put that to use to protect ourselves.
> > 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=112256
> > Fixes: 5f889b9a61dd ("drm/i915: Disregard drm_mode_config.fb_base")
> > References: da6c7707caf3 ("fbdev: Add FBINFO_HIDE_SMEM_START flag")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> > Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_fbdev.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > index 3d1061470e76..bff311561597 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > @@ -226,8 +226,8 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >               goto out_unpin;
> >       }
> >  
> > -     ifbdev->helper.fb = &ifbdev->fb->base;
> > -
> > +     /* don't leak any physical addresses to userspace */
> > +     info->flags |= FBINFO_HIDE_SMEM_START;
> 
> Doesn't the fb helper already do this?
> 
> Looks like it tries. Though I have no idea why it does
> it in initial_config() instead of fill_fb_info().

I wasn't even sure we took that path.

> >       info->fbops = &intelfb_ops;
> >  
> >       /* setup aperture base/size for vesafb takeover */
> > @@ -247,6 +247,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >       info->fix.smem_start = (unsigned long)info->screen_base;
> 
> Isn't screen_base the virtual address? Why do we put that into
> smem_start?

I made the mistake of thinking it was only internally used. So I take
that as an implicit r-b for the fix and the igt proving that I was
dumb...
https://patchwork.freedesktop.org/series/69416/
https://patchwork.freedesktop.org/series/69421/
-Chris