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

Daniel Vetter
2015-08-25T07:05:30Z

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@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).

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.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation