linux-kernel - Re: [RFC PATCH] drm:- Add a modifier to denote 'protected' framebuffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190917125301.GQ3958@phenom.ffwll.local>
Date:   Tue, 17 Sep 2019 14:53:01 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Ayan Halder <Ayan.Halder@....com>
Cc:     Liviu Dudau <Liviu.Dudau@....com>,
        Brian Starkey <Brian.Starkey@....com>,
        "maarten.lankhorst@...ux.intel.com" 
        <maarten.lankhorst@...ux.intel.com>,
        "maxime.ripard@...tlin.com" <maxime.ripard@...tlin.com>,
        "sean@...rly.run" <sean@...rly.run>,
        "airlied@...ux.ie" <airlied@...ux.ie>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        nd <nd@....com>
Subject: Re: [RFC PATCH] drm:- Add a modifier to denote 'protected'
 framebuffer

On Mon, Sep 09, 2019 at 01:42:53PM +0000, Ayan Halder wrote:
> Add a modifier 'DRM_FORMAT_MOD_ARM_PROTECTED' which denotes that the framebuffer
> is allocated in a protected system memory.
> Essentially, we want to support EGL_EXT_protected_content in our komeda driver.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.halder@....com>
> 
> /-- Note to reviewer
> Komeda driver is capable of rendering DRM (Digital Rights Management) protected
> content. The DRM content is stored in a framebuffer allocated in system memory
> (which needs some special hardware signals for access).
> 
> Let us ignore how the protected system memory is allocated and for the scope of
> this discussion, we want to figure out the best way possible for the userspace
> to communicate to the drm driver to turn the protected mode on (for accessing the
> framebuffer with the DRM content) or off.
> 
> The possible ways by which the userspace could achieve this is via:-
> 
> 1. Modifiers :- This looks to me the best way by which the userspace can
> communicate to the kernel to turn the protected mode on for the komeda driver
> as it is going to access one of the protected framebuffers. The only problem is
> that the current modifiers describe the tiling/compression format. However, it
> does not hurt to extend the meaning of modifiers to denote other attributes of
> the framebuffer as well.
> 
> The other reason is that on Android, we get an info from Gralloc
> (GRALLOC_USAGE_PROTECTED) which tells us that the buffer is protected. This can
> be used to set up the modifier/s (AddFB2) during framebuffer creation.

How does this mesh with other modifiers, like AFBC? That's where I see the
issue here.
> 
> 2. Framebuffer flags :- As of today, this can be one of the two values
> ie (DRM_MODE_FB_INTERLACED/DRM_MODE_FB_MODIFIERS). Unlike modifiers, the drm
> framebuffer flags are generic to the drm subsystem and ideally we should not
> introduce any driver specific constraint/feature.
> 
> 3. Connector property:- I could see the following properties used for DRM
> protected content:-
> DRM_MODE_CONTENT_PROTECTION_DESIRED / ENABLED :- "This property is used by
> userspace to request the kernel protect future content communicated over
> the link". Clearly, we are not concerned with the protection attributes of the
> transmitter. So, we cannot use this property for our case.
> 
> 4. DRM plane property:- Again, we want to communicate that the framebuffer(which
> can be attached to any plane) is protected. So introducing a new plane property
> does not help.
> 
> 5. DRM crtc property:- For the same reason as above, introducing a new crtc
> property does not help.

6. Just track this as part of buffer allocation, i.e. I think it does
matter how you allocate these protected buffers. We could add a "is
protected buffer" flag at the dma_buf level for this.

So yeah for this stuff here I think we do want the full userspace side,
from allocator to rendering something into this protected buffers (no need
to also have the entire "decode a protected bitstream part" imo, since
that will freak people out). Unfortunately, in my experience, that kills
it for upstream :-/ But also in my experience of looking into this for
other gpu's, we really need to have the full picture here to make sure
we're not screwing this up.
-Daniel

> 
> --/
> 
> ---
>  include/uapi/drm/drm_fourcc.h | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..38e5e81d11fe 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -742,6 +742,15 @@ extern "C" {
>   */
>  #define AFBC_FORMAT_MOD_BCH     (1ULL << 11)
>  
> +/*
> + * Protected framebuffer
> + *
> + * The framebuffer is allocated in a protected system memory which can be accessed
> + * via some special hardware signals from the dpu. This is used to support
> + * 'GRALLOC_USAGE_PROTECTED' in our framebuffer for EGL_EXT_protected_content.
> + */
> +#define DRM_FORMAT_MOD_ARM_PROTECTED	fourcc_mod_code(ARM, (1ULL << 55))
> +
>  /*
>   * Allwinner tiled modifier
>   *
> -- 
> 2.23.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch