[igt-dev] [PATCH i-g-t] tests/core_unauth_vs_render: new test for the relaxed DRM_AUTH handling

[Emil Velikov]

On Wed, 23 Jan 2019 at 11:33, Daniel Vetter <daniel at ffwll.ch> wrote:

> > > How do you envision the card* to renderD* helper? Should I go with the
> > > minor(rdev) & 0x80 hack, use the libdrm helpers or something else?
> >
> > I think it's minor(rdev) + 0x80, but yeah I think that's official uapi now
> > :-/
> >
Fwiw: The libdrm helpers use something else have been around for ages.
Mesa uses them not sure for others :-(

That said, I could remove all the crazy libdrm code and use
minor(rdev) + 0x80. Are you OK with that?

> > > A simple drm_open_driver() opens /dev/dri/card0 twice, when using the
> > > drm_open_driver_master we end up with an extra open().
> > > The kernel drm_getclient() was hollowed-out severely (commit
> > > 719524df4a2e48fa7ca3ad1697fd9a7f85ec8ad3), so I'm not sure if/how
> > > reliably we can use that.
> >
> > I wrote that igt to validate the hollowed-out getclient, and it's used by
> > libva (somehow all these hacks are used by libva) to check whether it's
> > authenticated. So should be accurate I hope.
> >
Code works fine, but have butchered my earlier testing. Sorry about the noise.

> > I guess if you want to check for -EACCESS make a 2nd test (or merge them)
> > which makes sure that if we have a non-render node, then we get an
> > EACCESS. Would mean a quick testrun on a display-only drm driver (those
> > are all non-rendernode) to make sure it works. And would be an even better
> > test I think.
This 2nd test is exactly what I wrote with this patch, is it not?
Admittedly it's missing the !DRM_CAP_PRIME case, which I will fix for
v2.

If we want to check that FD2HANDLE returns EBADF for invalid fd (-1 or
otherwise) - sure, but that's unrelated to the goal here.

>
> I just realized that as soon as you open a file as root you're
> authenticated, so this tests nothing. I think we need a new
> drm_open_any_nonroot(), which drops CAP_SYS_ADMIN (while otherwise
> staying as root, can't open the mkdev otherwise I think) and then restores
> it. Or something like this. Tbh not quite sure how to make this happen
> exactly.
>
> Noticed while I was trying to create a new check_auth() subtest for
> non-root not being authenticated from the start, and it didn't fail like I
> thought it should :-)
>
> So a simple
>
>         drm_open_any();
>         igt_drop_root();
>
> Doesn't cut it unfortunately :-/

Is that an assumption or you've tested the patch?

Just ran the patch over a dozen times [today alone] checking via
check_auth() and igt_debugfs_dump().
Admittedly a $chmod -R +rx /sys/kernel/debug was needed.

Did you miss the drm_open_any() call _after_ igt_drop_root() or I'm
really lucky somehow?

Thanks
Emil