Re: [PATCH 1/2] drm/core: Preserve the framebuffer after removing it.

Tvrtko Ursulin
2015-09-22T15:21:10Z

On 09/22/2015 03:53 PM, David Herrmann wrote:
> Hi
>
> On Thu, Sep 10, 2015 at 12:15 PM, Tvrtko Ursulin
> <tvrtko.ursulin@linux.intel.com> wrote:
>> On 09/10/2015 10:56 AM, Daniel Vetter wrote:
>>> That's not different from the compositor just freezing instead of
>>> crashing: Screen contents stays on and nothing happens. Imo this really is
>>> all just broken userspace, and the kernel can't make sure userspace
>>> doesn't randomly fall over.
>>>
>>> What we need to make sure is that assuming things work ok-ish there's no
>>> observed regression. And I still think that's the case here.
>>
>>
>> I would disagree on the no regressions when things work okay-ish principle,
>> there should be no regressions in the pessimistic scenario when security is
>> concerned.
>>
>> If we can agree the stuck frame on screen is not desirable from the security
>> point of view, then this change does enlarge the attack surface.
>>
>> Because, apart from freezing the compositor, it now also works to crash it
>> and prevent restart. Maybe it is far fetched, but as I said, attackers have
>> much better imagination with these things.
>
> I'd much more prefer a FB flag that forces a modeset on ID removal.
> Anyone who cares for it can set it, and the kernel will make sure to
> never keep such FBs around. However, for most use-cases, we want the
> FB to stay around after close() or FB removal.
>
> Btw., I also don't see the attack scenario at all. If an attacker
> makes your compositor crash at the same moment a security-relevant
> information is shown on screen, then the information is already
> visible. Who cares that it stays visible? Sure, I can make up an
> hypothetical use-case where that matters, but I cannot think of
> something real.
> Also, if the hypothetical scenario we go for is "if the compositor
> crashes", then I guess there's bigger problems than the FB content.

It allows losing control of the sensitive information in a new way and
so by definition results in a less secure system.

Apparently for the goal of less flicker and easier userspace
programming. Ie. avoiding the need for a back channel, apart from the
fact back channel has now just been moved to the kernel.

I would recommend passing this by some more security conscious eyes.

Regards,

Tvrtko