Bug 231221 - graphics/drm-devel-kmod: Make sure to allow only appropriate ioctl requests
Summary: graphics/drm-devel-kmod: Make sure to allow only appropriate ioctl requests
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Johannes M Dieterich
Depends on:
Reported: 2018-09-07 13:11 UTC by Tamas Szakaly
Modified: 2019-07-10 08:48 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (jmd)

Patch to reject inappropriate ioctls with EINVAL (423 bytes, text/plain)
2018-09-07 13:11 UTC, Tamas Szakaly
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tamas Szakaly 2018-09-07 13:11:46 UTC
Created attachment 196939 [details]
Patch to reject inappropriate ioctls with EINVAL


The drm_ioctl function (drivers/gpu/drm/drm_ioctl.c#783) uses DRM_IOCTL_NR(cmd) to determine which ioctl should be called, but it does not check if cmd is actually a valid ioctl for the driver.

Steps to Reproduce

I was trying to determine if a given file descriptor belongs to a drm or an input device, for which I was using two ioctl requests: IOCGVERSION and DRM_IOCTL_VERSION. I expected requesting IOCGVERSION from a drm device would return EINVAL, but it returns 0. This simple PoC demonstrates this behavior:

#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>
#include <sys/ioctl.h>
#include <dev/evdev/input.h>

int main(int argc, char** argv) {
        int fd = open("/dev/drm/0", O_RDONLY);
        int dummy = -1;
        int ret = ioctl(fd, EVIOCGVERSION, &dummy);
        printf("ret=%d, err=%s, dummy=%d\n", ret, strerror(errno), dummy);

Actual Results

The above program shows that requesting EVIOCGVERSION from a drm device succeeds:

[0x00 ~]$ cc test2.c -o test2
[0x00 ~]$ ./test2
ret=0, err=No error: 0, dummy=0

Expected Results

The expected result would be a failed ioctl call:

[0x00 ~]$ ./test2
ret=-1, err=Invalid argument, dummy=-1


This problem exists because DRM_IOCTL_NR(EVIOCGVERSION) = 1, therefore drm_ioctl will use DRM_IOCTL_GET_UNIQUE (because DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE) is also 1). The drm_ioctl.c in base compares IOCGROUP(cmd) with DRM_IOCTL_BASE, and returns -EINVAL if they differ. I have copied that comparison to the patch attached.
Comment 1 Johannes M Dieterich freebsd_committer 2018-09-16 03:49:06 UTC
Thanks for the patch! We are trying to keep the ports themselves patch-free, could you submit a PR against our githubs to ensure that your work also gets carried forward in future DRM imports?
Comment 2 Johannes M Dieterich freebsd_committer 2018-12-26 21:33:30 UTC
Has this been included? Can this be closed?
Comment 3 shadow53+freebsd 2019-06-09 02:20:10 UTC
(In reply to Johannes M Dieterich from comment #2)

Looks like it got merged upstream: https://github.com/FreeBSDDesktop/kms-drm/pull/93
Comment 4 Niclas Zeising freebsd_committer 2019-07-10 08:48:30 UTC
This looks like it has been fixed.  Please re-open this or create a new PR if this is still an issue.