Commit 01d3434a authored by Chris Wilson's avatar Chris Wilson Committed by Daniel Vetter
Browse files

drm: Don't overwrite user ioctl arg unless requested 



Currently, we completely ignore the user when it comes to the in/out
direction of the ioctl argument, as we simply cannot trust userspace.
(For example, they might request a copy of the modified ioctl argument
when the driver is not expecting such and so leak kernel stack.)
However, blindly copying over the target address may also lead to a
spurious EFAULT, and a failure after the ioctl was completed
successfully. This is important in order to avoid an ABI break when
extending an ioctl from IOR to IORW. Similar to how we only copy the
intersection of the kernel arg size and the user arg size, we only want
to copy back the kernel arg data iff both the kernel and userspace
request the copy.

Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: default avatarChristian König <christian.koenig@amd.com>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1468335590-21023-1-git-send-email-chris@chris-wilson.co.uk
parent 31954660

drivers/gpu/drm/drm_ioctl.c

+22 −28
Original line number Diff line number Diff line
@@ -648,7 +648,7 @@ long drm_ioctl(struct file *filp, 
	int retcode = -EINVAL;

	char stack_kdata[128];

	char *kdata = NULL;

	unsigned int usize, asize, drv_size;

	unsigned int in_size, out_size, drv_size, ksize;

	bool is_driver_ioctl;



	dev = file_priv->minor->dev;
@@ -671,9 +671,12 @@ long drm_ioctl(struct file *filp, 
	}



	drv_size = _IOC_SIZE(ioctl->cmd);

	usize = _IOC_SIZE(cmd);

	asize = max(usize, drv_size);

	cmd = ioctl->cmd;

	out_size = in_size = _IOC_SIZE(cmd);

	if ((cmd & ioctl->cmd & IOC_IN) == 0)

		in_size = 0;

	if ((cmd & ioctl->cmd & IOC_OUT) == 0)

		out_size = 0;

	ksize = max(max(in_size, out_size), drv_size);



	DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",

		  task_pid_nr(current),
@@ -693,29 +696,23 @@ long drm_ioctl(struct file *filp, 
	if (unlikely(retcode))

		goto err_i1;



	if (cmd & (IOC_IN | IOC_OUT)) {

		if (asize <= sizeof(stack_kdata)) {

	if (ksize <= sizeof(stack_kdata)) {

		kdata = stack_kdata;

	} else {

			kdata = kmalloc(asize, GFP_KERNEL);

		kdata = kmalloc(ksize, GFP_KERNEL);

		if (!kdata) {

			retcode = -ENOMEM;

			goto err_i1;

		}

	}

		if (asize > usize)

			memset(kdata + usize, 0, asize - usize);

	}



	if (cmd & IOC_IN) {

		if (copy_from_user(kdata, (void __user *)arg,

				   usize) != 0) {

	if (copy_from_user(kdata, (void __user *)arg, in_size) != 0) {

		retcode = -EFAULT;

		goto err_i1;

	}

	} else if (cmd & IOC_OUT) {

		memset(kdata, 0, usize);

	}



	if (ksize > in_size)

		memset(kdata + in_size, 0, ksize - in_size);



	/* Enforce sane locking for kms driver ioctls. Core ioctls are

	 * too messy still. */
@@ -728,11 +725,8 @@ long drm_ioctl(struct file *filp, 
		mutex_unlock(&drm_global_mutex);

	}



	if (cmd & IOC_OUT) {

		if (copy_to_user((void __user *)arg, kdata,

				 usize) != 0)

	if (copy_to_user((void __user *)arg, kdata, out_size) != 0)

		retcode = -EFAULT;

	}



      err_i1:

	if (!ioctl)

CRA Git | Maintained and supported by SUSTech CRA and CCSE