| Message ID | 20191113171944.19308-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/fbdev: Hide smem_start from userspace | expand |
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@chris-wilson.co.uk> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Maarten Lankhorst <maarten.lankhorst@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(). > 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? > info->fix.smem_len = info->screen_size; > > + ifbdev->helper.fb = &ifbdev->fb->base; > drm_fb_helper_fill_info(info, &ifbdev->helper, sizes); > > /* If the object is shmemfs backed, it will have given us zeroed pages. > -- > 2.24.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
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@chris-wilson.co.uk> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch> > > Cc: Maarten Lankhorst <maarten.lankhorst@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
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; 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; info->fix.smem_len = info->screen_size; + ifbdev->helper.fb = &ifbdev->fb->base; drm_fb_helper_fill_info(info, &ifbdev->helper, sizes); /* If the object is shmemfs backed, it will have given us zeroed pages.
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@chris-wilson.co.uk> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> --- drivers/gpu/drm/i915/display/intel_fbdev.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)