On 11/01/2023 22:19, Daniel Vetter wrote:
On Tue, Jan 10, 2023 at 01:14:51PM +0000, Tvrtko Ursulin wrote:
On 06/01/2023 18:00, Daniel Vetter wrote:
On Fri, Jan 06, 2023 at 03:53:13PM +0100, Christian König wrote:
Am 06.01.23 um 11:53 schrieb Daniel Vetter:
On Fri, Jan 06, 2023 at 11:32:17AM +0100, Christian König wrote:
Am 05.01.23 um 13:32 schrieb Daniel Vetter:
[SNIP]
For the case of an master fd I actually don't see the reason why we should
limit that? And fd can become master if it either was master before or has
CAP_SYS_ADMIN. Why would we want an extra check for the pid/tgid here?
This is just info/debug printing, I don't see the connection to
drm_auth/master stuff? Aside from the patch mixes up the master opener and
the current user due to fd passing or something like that.
That's exactly why my comment meant as well.
The connect is that the drm_auth/master code currently the pid/tgid as
indicator if the "owner" of the fd has changed and so if an access should be
allowed or not. I find that approach a bit questionable.
Note that we cannot do that (I think at least, after pondering this some
more) because it would break the logind master fd passing scheme - there
the receiving compositor is explicitly _not_ allowed to acquire master
rights on its own. So the master priviledges must not move with the fd or
things can go wrong.
That could be the rational behind that, but why doesn't logind then just
pass on a normal render node to the compositor?
Because the compositor wants the kms node. We have 3 access levels in drm
- render stuff
- modeset stuff (needs a card* node and master rights for changing things)
- set/drop master (needs root)
logind wants to give the compositor modeset access, but not master
drop/set access (because vt switching is controlled by logind).
The pid check in drm_auth is for the use-case where you start your
compositor on a root vt (or setuid-root), and then want to make sure
that after cred dropping, set/drop master keeps working. Because in that
case the vt switch dance is done by the compositor.
Maybe we should document this stuff a bit better :-)
Maybe add a friendly warning? E.g. like "Don't touch it, it works!" :)
I think Tvrtko just volunteered for that :-) Maybe addition in the
drm-uapi.rst section would be good that fills out the gaps we have.
I can attempt to copy, paste and tidy what you wrote here, albeit with less
than full degree of authority. Assuming into the existing comment above
drm_master_check_perm?
I'd put it into the DOC: section in drm_auth.c so it shows up in the
drm-uapi.rst docs. Or do a new one if you want to split this out and then
include it in the drm-uapi.rst.
But in terms of where my series is going next I would need some
clarification in the other sub-thread.
Maybe I'm lost on what the leftover confusion is? This one seemed to be
it?
The question of whether you are now okay with my approach to track
file_priv->pid if !was_master, or your counter-proposal to have
file_priv->pid and file_priv->"render_user_pid" is still relevant.
If the latter then what semantics have been settled at, one-shot
transition or not?
I had an issue with one-shot and even didn't fully understand what you
did not like about my proposal.
Regards,
Tvrtko
