[PATCH 2/3] drm:msm: Initial Add Writeback Support (V2)

[Daniel Vetter]

On Tue, Aug 25, 2015 at 9:11 PM, Rob Clark <robdclark at gmail.com> wrote:
> On Tue, Aug 25, 2015 at 3:05 AM, Daniel Vetter <daniel at ffwll.ch> wrote:
>> On Wed, Aug 19, 2015 at 03:00:04PM -0400, Rob Clark wrote:
>>> On Tue, Apr 7, 2015 at 2:09 PM, Jilai Wang <jilaiw at codeaurora.org> wrote:
>>> So one thing that I wanted sorting out before we let userspace see
>>> streaming writeback (where I do think v4l is the right interface), is
>>> a way to deal w/ permissions/security..  Ie. only the kms master
>>> should control access to writeback.  Ie. an process that the
>>> compositor isn't aware of / doesn't trust, should not be able to open
>>> the v4l device and start snooping on the screen contents.  And I don't
>>> think just file permissions in /dev is sufficient.  You likely don't
>>> want to run your helper process doing video encode and streaming as a
>>> privilaged user.
>>>
>>> One way to handle this would be some sort of dri2 style
>>> getmagic/authmagic sort of interface between the drm/kms master, and
>>> v4l device, to unlock streaming.  But that is kind of passe.  Fd
>>> passing is the fashionable thing now.  So instead we could use a dummy
>>> v4l2_file_opererations::open() which always returns an error.  So v4l
>>> device shows up in /dev.. but no userspace can open it.  And instead,
>>> the way to get a fd for the v4l dev would be via a drm/kms ioctl (with
>>> DRM_MASTER flag set).  Once compositor gets the fd, it can use fd
>>> passing, if needed, to hand it off to a helper process, etc.
>>>
>>> (probably use something like alloc_file() to get the 'struct file *',
>>> then call directly into v4l2_fh_open(), and then get_unused_fd_flags()
>>> + fd_install() to get fd to return to userspace)
>>
>> Just following up here, but consensus from the lpc track is that we don't
>> need this as long as writeback is something which needs a specific action
>> to initiate. I.e. if we have a separate writeback connector which won't
>> grab any frames at all as long as it's disconnected we should be fine. Wrt
>> session handling that's something which would need to be fixed between
>> v4l and logind (or whatever).
>
> Was that consensus?  I agree that something should initiate writeback
> from the kms side of things.  But if we don't have *something* to
> ensure whatever is on the other end of writeback is who we think it
> is, it seems at least racy..
>
>> In general I don't like hand-rolling our own proprietary v4l-open process,
>> it means that all the existing v4l test&dev tooling must be changed, even
>> when you're root.
>
> well, I know that, for example, gst v4l2src allows you to pass in an
> already opened v4l dev fd, which fits in pretty well with what I
> propose.  The alternative, I think, is a dri2 style auth handshake
> between drm/kms and v4l, which I am less thrilled about.
>
> I would have to *assume* that userspace is at least prepared to deal
> with -EPERM when it tries to open a device..  at least more so than
> special auth ioctl sequence.

Imo the right approach for that is to defer to logind. We already have
these issues, e.g. you probably don't want the wrong seat to be able
to access camera devices. Maybe no one bothered to support this yet,
but the problem is definitely there already. And with that broder
use-case of just making sure the right process can access the v4l
device nodes drm writeback is just another special case.

At least that's what I wanted to say at lpc, might have done a bad job
at phrasing it ;-)

DRM auth dance otoh is something entirely different and only needed
because we want multiple processes to use the same device. v4l doesn't
support that model yet at all, it really only needs some form of
revoke + logind adjusting permissions to match whatever the current
user is on a given seat.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch