On Fri, Jul 16, 2021 at 05:43:12PM +0100, Charles Baylis wrote: > > Hi > > The attached patch fixes a problem where non-drm ioctls are incorrectly > handled by drm drivers. > > This causes problems when isatty() is called on a file descriptor which > was opened on a drm device node. Glibc implements isatty() by invoking > the TCGETS ioctl on the fd. TCGETS is 0x5401, so this is handled by drm_ioctl > as DRM_IOCTL_GET_UNIQUE, which succeeds, so isatty() returns true. > > As a side effect, DRM_IOCTL_GET_UNIQUE also writes to a pointer, in the > argument buffer, so the calling application's memory can be corrupted, causing > a crash later. > > Tested on an Ubuntu 20.10 VM under qemu with virgl: > * "if [ -t 0 ]; then echo is a tty; fi < /dev/dri/card0" outputs nothing > * glxgears still works > > Thanks > Charles > commit 0072b12086cdf7350df75d36a12d392e3ba92865 > Author: Charles Baylis <cb-kernel@xxxxxxxxxxxxx> > Date: Fri Jul 16 11:58:47 2021 +0100 > > drm: Return -ENOTTY for non-drm ioctls > > Return -ENOTTY from drm_ioctl() when userspace passes in a cmd number > which doesn't relate to the drm subsystem. > > Glibc uses the TCGETS ioctl to implement isatty(), and without this > change isatty() returns it incorrectly returns true for drm devices. > > To test run this command: > $ if [ -t 0 ]; then echo is a tty; fi < /dev/dri/card0 > which shows "is a tty" without this patch. > > This may also modify memory which the userspace application is not > expecting. I fixed up the formatting here a bit to get git apply-mbox to accept this, please try to use git send-email. Anyway this is bug is as old as drm, did you see whether other subsystems are buggy like this too? Patch applied to drm-misc-fixes with cc: stable. Since this is pretty blantant issue, care to also type up an igt tests to verify this stays fixed? https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#validating-changes-with-igt Cheers, Daniel > > Signed-off-by: Charles Baylis <cb-kernel@xxxxxxxxxxxxx> > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c > index 98ae00661656..f454e0424086 100644 > --- a/drivers/gpu/drm/drm_ioctl.c > +++ b/drivers/gpu/drm/drm_ioctl.c > @@ -834,6 +834,9 @@ long drm_ioctl(struct file *filp, > if (drm_dev_is_unplugged(dev)) > return -ENODEV; > > + if (DRM_IOCTL_TYPE(cmd) != DRM_IOCTL_BASE) > + return -ENOTTY; > + > is_driver_ioctl = nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END; > > if (is_driver_ioctl) { > diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h > index 10100a4bbe2a..afb27cb6a7bd 100644 > --- a/include/drm/drm_ioctl.h > +++ b/include/drm/drm_ioctl.h > @@ -68,6 +68,7 @@ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd, > unsigned long arg); > > #define DRM_IOCTL_NR(n) _IOC_NR(n) > +#define DRM_IOCTL_TYPE(n) _IOC_TYPE(n) > #define DRM_MAJOR 226 > > /** -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
