From: Keith Packard <keithp@keithp.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: linux-kernel@vger.kernel.org, Dave Airlie <airlied@redhat.com>,
        dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 3/4] drm: Check mode object lease status in all master ioctl paths
In-Reply-To: <20170402131926.gejo4wnicwwekgqk@phenom.ffwll.local>
References: <20170401170841.2643-1-keithp@keithp.com> <20170401170841.2643-4-keithp@keithp.com> <20170402131926.gejo4wnicwwekgqk@phenom.ffwll.local>
Date: Sun, 02 Apr 2017 09:37:16 -0700
Message-ID: <864ly6kbpf.fsf@hiro.keithp.com>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
        micalg=pgp-sha256; protocol="application/pgp-signature"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2772
Lines: 66

--=-=-=
Content-Type: text/plain
Content-Transfer-Encoding: quoted-printable

Daniel Vetter <daniel@ffwll.ch> writes:

> I think it'd be good if we could consolidate all the lease checking into
> drm_mode_object_find (respectively __drm_mode_object_find). We'd need to
> wire up the fpriv to be able to do that, but we could upstream that patch
> right away before anything else. That should take care of most of the
> checks in this patch here.

That's a good idea.

> There's a few things on top:
> - filtering the various bitmasks. I think you have most, but we could
>   perhaps upstream the helpers for these.

Yeah, would be nice to get hooks in place soon to avoid rebase
adventures later. I guess that would involve shipping a stub drm_lease.h
for now?

> - filtering object lists (essentially getresources and getplanes ioctls).
> - filtering implicit objects in the legacy ioctl. E.g. page_flip done
>   through atomic doesn't just need the CRTC id, but also the id of the
>   primary plane plus of the FB_ID atomic property. Similarly for all the
>   other legacy ioctls. I think we want to make sure there's no difference
>   here in behaviour.

Oh, all of the implicit resource access from the legacy ioctls. Yeah,
that will take a bit of research to identify all of them.

> Especially for the last one it might be simplest to outright disallow all
> legacy ioctl and require that sub-drm_master nodes only get access to the
> read-only GET* ioctl (they get that anyway, even when they're not the
> current master), plus atomic. Makes it a _lot_ easier to implement.
> Downside is that amdgpu _really_ needs to land atomic asap :-)

I'd like to avoid that particular dependency as amdgpu is something of a
requirement for this particular project...

I'll get started fixing the lease checking stuff to try and centralize it.

=2D-=20
=2Dkeith

--=-=-=
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEw4O3eCVWE9/bQJ2R2yIaaQAAABEFAljhKLwACgkQ2yIaaQAA
ABGJxA/9EYS3lxfXJ9Mldb14jF5fbIRDQ48T+o6UaBYb/t5mE414G/qMrhDbdYP5
1d3GaKSkVnXpO7eZ1LGxAl59fwh14NI1QCwa3Qo0F7yTRxeHOY/ORa7R6S5qNjh1
lUsonAsKozVEAopouzEM+dFRBfHKGc3afzEPF3/JnJvi10uXgpb0R/+3xfVG7Ms+
pEX/OW3rvCHRPeTmwf5sX5/Hn9I9mGuNnaXWYjWOxKkyfERDrMvzN7m0FFjFvmLh
9KbYRLrd//bkh5l87DdVT97ThYWC6pYL1FTqK5Qk04SBk3BQTT/2V1pcMX2rChSc
HnlZ5WUBDTWQtTk8fW5kTak7FviZ9MYLKp7TZvLqGDr589EDiyfwMPEkL6m0fUiH
glWk/VJngZOv80IWLJxbejS+3+EAzgkaSelxzYJ7Zz6bEOFX69p3XcCdFwpMX1LI
cJU/ggJDmFj6KYijA2+KItTAJgRkK1SG8aWeStheuA4BU0hPwegT51kjeJHSAIbx
oOXNAu4lo7+mO8v80nYIEgqnsykDlOMrflUbCl3RtPXv0Biex8LN9Hh6zSounmeP
c/+r0thO6lqg/zepKbdx6JpTZktC8ODwBdCuxyzocZE3cB62Win9epch6Rqk8xqu
PJX1io6/ER419y8xDx45NT9tzuE++Z48tEQxIkEXJ+2bMvaWdOo=
=fSrW
-----END PGP SIGNATURE-----
--=-=-=--