On Friday 30 October 2009, Dave Airlie wrote:
> Btw when I mentioned ioctls I meant more than radeon, all the KMS
> ioctls in the common drm_crtc.c file suffer from this problem as well.
>
> Hence why I still believe either my drm specific inline or something
> more generic (granted I can see why a generic solution would be ugly).
>
> You patch below does suffer from a lot of #ifdefs and cut-n-paste
> that is a lot better suited to doing in an inline or macro. We can then
> comment that inline saying if anyone else does this we will be most
> unhappy.
I think it would be better to do a conversion of the pointers in
a separate compat handler, but then call the regular function, which
in case of drm already take a kernel pointer. That would be much simpler
than the compat_alloc_user_space tricks in the current code and
also cleaner than trying to handle both cases in one function using
is_compat_task().
See the (not even compile-tested) example below as an illustration
of what I think you should do. If you convert all functions in
drm_ioc32.c to this scheme, you can replace drm_compat_ioctl and
drm_compat_ioctls[] with drm_generic_compat_ioctl.
Arnd <><
diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c
index 282d9fd..334345b 100644
--- a/drivers/gpu/drm/drm_ioc32.c
+++ b/drivers/gpu/drm/drm_ioc32.c
@@ -1040,6 +1040,122 @@ static int compat_drm_wait_vblank(struct file *file,
unsigned int cmd,
return 0;
}
+static int compat_drm_mode_getblob_ioctl(struct drm_device *dev,
+ struct drm_mode_get_blob __user *out_resp_user,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_get_blob out_resp;
+ int ret;
+
+ ret = copy_from_user(&out_resp, out_resp_user, sizeof(out_resp))
+ if (ret)
+ return -EFAULT;
+
+ out_resp.data = (unsigned long)compat_ptr(out_resp.data);
+
+ ret = drm_mode_getblob_ioctl(dev, &out_resp, file_priv);
+ if (ret)
+ return ret;
+
+ ret = copy_to_user(out_resp_user, &out_resp, sizeof(out_resp))
+ if (ret)
+ return -EFAULT;
+ return 0;
+}
+
+static int compat_drm_mode_gamma_set_ioctl(struct drm_device *dev,
+ struct drm_mode_crtc_lut __user *crtc_lut_user,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_crtc_lut crtc_lut;
+
+ ret = copy_from_user(&crtc_lut, crtc_lut_user, sizeof(crtc_lut))
+ if (ret)
+ return -EFAULT;
+
+ crtc_lut.red = (unsigned long)compat_ptr(crtc_lut.red);
+ crtc_lut.green = (unsigned long)compat_ptr(crtc_lut.green);
+ crtc_lut.blue = (unsigned long)compat_ptr(crtc_lut.blue);
+
+ ret = drm_mode_gamma_set_ioctl(dev, &crtc_lut, file_priv);
+
+ return ret;
+}
+
+static int compat_drm_mode_gamma_get_ioctl(struct drm_device *dev,
+ struct drm_mode_crtc_lut __user *crtc_lut_user,
+ struct drm_file *file_priv)
+{
+ struct drm_mode_crtc_lut crtc_lut;
+
+ ret = copy_from_user(&crtc_lut, crtc_lut_user, sizeof(crtc_lut))
+ if (ret)
+ return -EFAULT;
+
+ crtc_lut.red = (unsigned long)compat_ptr(crtc_lut.red);
+ crtc_lut.green = (unsigned long)compat_ptr(crtc_lut.green);
+ crtc_lut.blue = (unsigned long)compat_ptr(crtc_lut.blue);
+
+ ret = drm_mode_gamma_get_ioctl(dev, &crtc_lut, file_priv);
+
+ return ret;
+}
+
+static int drm_generic_compat_ioctl(struct file *filp, unsigned int cmd,
+ unsigned long arg)
+{
+ struct drm_file *file_priv = filp->private_data;
+ struct drm_device *dev = file_priv->minor->dev;
+ unsigned int nr = DRM_IOCTL_NR(cmd);
+ int ret;
+
+ atomic_inc(&dev->ioctl_count);
+ atomic_inc(&dev->counts[_DRM_STAT_IOCTLS]);
+ ++file_priv->ioctl_count;
+
+ if ((nr >= DRM_CORE_IOCTL_COUNT) &&
+ ((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
+ goto err_i1;
+ if ((nr >= DRM_COMMAND_BASE) && (nr < DRM_COMMAND_END) &&
+ (nr < DRM_COMMAND_BASE + dev->driver->num_ioctls))
+ ioctl = &dev->driver->ioctls[nr - DRM_COMMAND_BASE];
+ else if ((nr >= DRM_COMMAND_END) || (nr < DRM_COMMAND_BASE)) {
+ ioctl = &drm_ioctls[nr];
+ cmd = ioctl->cmd;
+ } else
+ goto err_i1;
+
+ if (((ioctl->flags & DRM_ROOT_ONLY) && !capable(CAP_SYS_ADMIN)) ||
+ ((ioctl->flags & DRM_AUTH) && !file_priv->authenticated) ||
+ ((ioctl->flags & DRM_MASTER) && !file_priv->is_master) ||
+ (!(ioctl->flags & DRM_CONTROL_ALLOW) &&
(file_priv->minor->type == DRM_MINOR_CONTROL))) {
+ ret = -EACCES;
+ goto err_il;
+ }
+
+ switch (cmd) {
+ case DRM_IOCTL_MODE_GETPROPBLOB:
+ ret = compat_drm_mode_getblob_ioctl(dev,
+ compat_ptr(arg), file_priv);
+ break;
+
+ case DRM_IOCTL_MODE_GETGAMMA:
+ ret = compat_drm_mode_gamma_set_ioctl(dev,
+ compat_ptr(arg), file_priv);
+ break;
+
+ case DRM_IOCTL_MODE_GETGAMMA:
+ ret = compat_drm_mode_gamma_get_ioctl(dev,
+ compat_ptr(arg), file_priv);
+ break;
+ }
+
+ err_i1:
+ atomic_dec(&dev->ioctl_count);
+
+ return ret;
+}
+
drm_ioctl_compat_t *drm_compat_ioctls[] = {
[DRM_IOCTL_NR(DRM_IOCTL_VERSION32)] = compat_drm_version,
[DRM_IOCTL_NR(DRM_IOCTL_GET_UNIQUE32)] = compat_drm_getunique,
@@ -1072,6 +1188,9 @@ drm_ioctl_compat_t *drm_compat_ioctls[] = {
[DRM_IOCTL_NR(DRM_IOCTL_UPDATE_DRAW32)] = compat_drm_update_draw,
#endif
[DRM_IOCTL_NR(DRM_IOCTL_WAIT_VBLANK32)] = compat_drm_wait_vblank,
+ [DRM_IOCTL_MODE_GETPROPBLOB] = drm_generic_compat_ioctl,
+ [DRM_IOCTL_MODE_GETGAMMA] = drm_generic_compat_ioctl,
+ [DRM_IOCTL_MODE_SETGAMMA] = drm_generic_compat_ioctl,
};
/**
------------------------------------------------------------------------------
Come build with us! The BlackBerry(R) Developer Conference in SF, CA
is the only developer event you need to attend this year. Jumpstart your
developing skills, take BlackBerry mobile applications to market and stay
ahead of the curve. Join us from November 9 - 12, 2009. Register now!
http://p.sf.net/sfu/devconference
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
