On Wed, Sep 09, 2015 at 04:47:08PM +0100, Tvrtko Ursulin wrote: > > On 09/09/2015 04:29 PM, Daniel Vetter wrote: > >On Wed, Sep 09, 2015 at 04:18:02PM +0100, Tvrtko Ursulin wrote: > >> > >>On 09/09/2015 04:04 PM, Daniel Vetter wrote: > >>>On Wed, Sep 09, 2015 at 03:51:50PM +0100, Tvrtko Ursulin wrote: > >>>> > >>>>Hi, > >>>> > >>>>On 09/09/2015 03:40 PM, Maarten Lankhorst wrote: > >>>>>Previously RMFB and fd close chose to disable any plane that had > >>>>>an active framebuffer from this file. If it was a primary plane the > >>>>>crtc was disabled. However the fbdev code or any system compositor > >>>>>should restore the planes anyway so there's no need to do it twice. > >>>>> > >>>>>The old fb_id is zero'd, so there's no danger of being able to > >>>>>restore the fb from fb_id. > >>>> > >>>>What does this mean, say if the compositor dies last frame will remain on > >>>>the screen? > >>> > >>>Yes, and the commit message should mention that. It should also mention > >>>that other applications can't get at the data since we clear fb id still, > >>>so no information leak there. > >> > >>Perhaps I replied to the wrong patch from the series. > >> > >>Why is all this needed anyway? It sound pretty undesirable from the security > >>point of view to me. If it is exploitable to leave something sensitive on > >>screen that's not good. > > > >fd close is a super-painful context to do a full-blown modeset. It's > >userspace but we can't restart anything because no one ever checks the > >return value of close(). We could fix it by pushing this to a work item, > >but given that the rule itself seems dubious it's easier to adjust the abi > >imo. Framebuffers are somewhat global, so not deleting them makes imo > >sense. > > > >The big change is patch 2, which will make them survive for real. > > I don't follow this closely but it still sounds wrong. If modeset is a > concern then disable the planes and/or clear them? This is generic core code, you can't disable/clear planes in a generic way without doing a full modeset. > It really doesn't feel preservation of fb content is a good thing to do. If > the higher goal is to enable some smooth transitions clients should engineer > that themselves. > > In any case leaving content on screen sounds really bad to me. > > Reminds me of screen locker bugs which sometimes did not clear the screen > when displaying the unlock dialog. That was pretty common for a long period > in KDE. And this sounds like it could be attackable in a similar way. Afaik that's just userspace coordination fail - system deamons go into suspend without telling and waiting for the screenlocker to lock the screen. Hilarious, but not really something we can fix in the kernel. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
