On 30/01/2023 14:50, carsten.haitzler@foss.arm.com wrote:
From: Carsten Haitzler carsten.haitzler@foss.arm.com
DRM compat code totally assumes 32bit is compat. This is not always
Still think that s/32bit is compat/compat is 32-bit/ makes more sense.
the case. This allows for compat to be defined universally to typedefed types like compat_ulong_t and so on. This converts some of the drm compat handling for ioctl's to use these types.
Signed-off-by: Carsten Haitzler Carsten.Haitzler@arm.com
drivers/gpu/drm/drm_ioc32.c | 209 ++++++++++++++++++------------------ include/drm/drm_ioctl.h | 2 +- 2 files changed, 106 insertions(+), 105 deletions(-)
diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index 5d82891c3222..13e7d25dcbae 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -38,21 +38,21 @@ #include "drm_internal.h" #include "drm_legacy.h" -#define DRM_IOCTL_VERSION32 DRM_IOWR(0x00, drm_version32_t) -#define DRM_IOCTL_GET_UNIQUE32 DRM_IOWR(0x01, drm_unique32_t) +#define DRM_IOCTL_VERSION_COMPAT DRM_IOWR(0x00, drm_version_compat_t) +#define DRM_IOCTL_GET_UNIQUE_COMPAT DRM_IOWR(0x01, drm_unique_compat_t)
I still think we're better off without the partial renaming, to be consistent both in that file and with what we've done so far (i.e. minimal changes if reasonably possible).
#define DRM_IOCTL_GET_MAP32 DRM_IOWR(0x04, drm_map32_t) -#define DRM_IOCTL_GET_CLIENT32 DRM_IOWR(0x05, drm_client32_t) -#define DRM_IOCTL_GET_STATS32 DRM_IOR( 0x06, drm_stats32_t) +#define DRM_IOCTL_GET_CLIENT_COMPAT DRM_IOWR(0x05, drm_client_compat_t) +#define DRM_IOCTL_GET_STATS_COMPAT DRM_IOR( 0x06, drm_stats_compat_t) -#define DRM_IOCTL_SET_UNIQUE32 DRM_IOW( 0x10, drm_unique32_t) -#define DRM_IOCTL_ADD_MAP32 DRM_IOWR(0x15, drm_map32_t) +#define DRM_IOCTL_SET_UNIQUE_COMPAT DRM_IOW( 0x10, drm_unique_compat_t) +#define DRM_IOCTL_ADD_MAP_COMPAT DRM_IOWR(0x15, drm_map_compat_t) #define DRM_IOCTL_ADD_BUFS32 DRM_IOWR(0x16, drm_buf_desc32_t) #define DRM_IOCTL_MARK_BUFS32 DRM_IOW( 0x17, drm_buf_desc32_t) #define DRM_IOCTL_INFO_BUFS32 DRM_IOWR(0x18, drm_buf_info32_t) #define DRM_IOCTL_MAP_BUFS32 DRM_IOWR(0x19, drm_buf_map32_t) #define DRM_IOCTL_FREE_BUFS32 DRM_IOW( 0x1a, drm_buf_free32_t) -#define DRM_IOCTL_RM_MAP32 DRM_IOW( 0x1b, drm_map32_t) +#define DRM_IOCTL_RM_MAP_COMPAT DRM_IOW( 0x1b, drm_map_compat_t) #define DRM_IOCTL_SET_SAREA_CTX32 DRM_IOW( 0x1c, drm_ctx_priv_map32_t) #define DRM_IOCTL_GET_SAREA_CTX32 DRM_IOWR(0x1d, drm_ctx_priv_map32_t) @@ -72,92 +72,92 @@ #define DRM_IOCTL_UPDATE_DRAW32 DRM_IOW( 0x3f, drm_update_draw32_t) -#define DRM_IOCTL_WAIT_VBLANK32 DRM_IOWR(0x3a, drm_wait_vblank32_t) +#define DRM_IOCTL_WAIT_VBLANK_COMPAT DRM_IOWR(0x3a, drm_wait_vblank_compat_t) #define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8, drm_mode_fb_cmd232_t) -typedef struct drm_version_32 { +typedef struct drm_version_compat { int version_major; /* Major version */ int version_minor; /* Minor version */ int version_patchlevel; /* Patch level */
- u32 name_len; /* Length of name buffer */
- u32 name; /* Name of driver */
- u32 date_len; /* Length of date buffer */
- u32 date; /* User-space buffer to hold date */
- u32 desc_len; /* Length of desc buffer */
- u32 desc; /* User-space buffer to hold desc */
-} drm_version32_t;
- compat_size_t name_len; /* Length of name buffer */
- compat_uptr_t name; /* Name of driver */
- compat_size_t date_len; /* Length of date buffer */
- compat_uptr_t date; /* User-space buffer to hold date */
- compat_size_t desc_len; /* Length of desc buffer */
- compat_uptr_t desc; /* User-space buffer to hold desc */
+} drm_version_compat_t; static int compat_drm_version(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){
- drm_version32_t v32;
- drm_version_compat_t v_compat; struct drm_version v; int err;
- if (copy_from_user(&v32, (void __user *)arg, sizeof(v32)))
- if (copy_from_user(&v_compat, (void __user *)arg, sizeof(v_compat))) return -EFAULT;
memset(&v, 0, sizeof(v)); v = (struct drm_version) {
.name_len = v32.name_len,.name = compat_ptr(v32.name),.date_len = v32.date_len,.date = compat_ptr(v32.date),.desc_len = v32.desc_len,.desc = compat_ptr(v32.desc),
.name_len = v_compat.name_len,.name = compat_ptr(v_compat.name),.date_len = v_compat.date_len,.date = compat_ptr(v_compat.date),.desc_len = v_compat.desc_len,.desc = compat_ptr(v_compat.desc),
- v32.version_major = v.version_major;
- v32.version_minor = v.version_minor;
- v32.version_patchlevel = v.version_patchlevel;
- v32.name_len = v.name_len;
- v32.date_len = v.date_len;
- v32.desc_len = v.desc_len;
- if (copy_to_user((void __user *)arg, &v32, sizeof(v32)))
- v_compat.version_major = v.version_major;
- v_compat.version_minor = v.version_minor;
- v_compat.version_patchlevel = v.version_patchlevel;
- v_compat.name_len = v.name_len;
- v_compat.date_len = v.date_len;
- v_compat.desc_len = v.desc_len;
- if (copy_to_user((void __user *)arg, &v_compat, sizeof(v_compat))) return -EFAULT; return 0;
} -typedef struct drm_unique32 {
- u32 unique_len; /* Length of unique */
- u32 unique; /* Unique name for driver instantiation */
-} drm_unique32_t; +typedef struct drm_unique_compat {
- compat_size_t unique_len; /* Length of unique */
- compat_uptr_t unique; /* Unique name for driver instantiation */
+} drm_unique_compat_t; static int compat_drm_getunique(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){
- drm_unique32_t uq32;
- drm_unique_compat_t uq_compat; struct drm_unique uq; int err;
- if (copy_from_user(&uq32, (void __user *)arg, sizeof(uq32)))
- if (copy_from_user(&uq_compat, (void __user *)arg, sizeof(uq_compat))) return -EFAULT;
memset(&uq, 0, sizeof(uq)); uq = (struct drm_unique){
.unique_len = uq32.unique_len,.unique = compat_ptr(uq32.unique),
.unique_len = uq_compat.unique_len,.unique = compat_ptr(uq_compat.unique),err = drm_ioctl_kernel(file, drm_getunique, &uq, 0); if (err) return err;
- uq32.unique_len = uq.unique_len;
- if (copy_to_user((void __user *)arg, &uq32, sizeof(uq32)))
- uq_compat.unique_len = uq.unique_len;
- if (copy_to_user((void __user *)arg, &uq_compat, sizeof(uq_compat))) return -EFAULT; return 0;
} static int compat_drm_setunique(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){ /* it's dead */ return -EINVAL; @@ -167,14 +167,14 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd, typedef struct drm_map32 { u32 offset; /* Requested physical address (0 for SAREA) */ u32 size; /* Requested physical size (bytes) */
- enum drm_map_type type; /* Type of memory to map */
- enum drm_map_type type; /* Type of memory to map */ enum drm_map_flags flags; /* Flags */ u32 handle; /* User-space: "Handle" to pass to mmap() */
- int mtrr; /* MTRR slot used */
- int mtrr; /* MTRR slot used */
} drm_map32_t; static int compat_drm_getmap(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){ drm_map32_t __user *argp = (void __user *)arg; drm_map32_t m32; @@ -193,7 +193,7 @@ static int compat_drm_getmap(struct file *file, unsigned int cmd, m32.size = map.size; m32.type = map.type; m32.flags = map.flags;
- m32.handle = ptr_to_compat((void __user *)map.handle);
- m32.handle = ptr_to32((void __user *)map.handle); m32.mtrr = map.mtrr; if (copy_to_user(argp, &m32, sizeof(m32))) return -EFAULT;
@@ -202,7 +202,7 @@ static int compat_drm_getmap(struct file *file, unsigned int cmd, } static int compat_drm_addmap(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){ drm_map32_t __user *argp = (void __user *)arg; drm_map32_t m32; @@ -224,7 +224,7 @@ static int compat_drm_addmap(struct file *file, unsigned int cmd, m32.offset = map.offset; m32.mtrr = map.mtrr;
- m32.handle = ptr_to_compat((void __user *)map.handle);
- m32.handle = ptr_to32((void __user *)map.handle); if (map.handle != compat_ptr(m32.handle)) pr_err_ratelimited("compat_drm_addmap truncated handle %p for type %d offset %x\n", map.handle, m32.type, m32.offset);
@@ -236,11 +236,11 @@ static int compat_drm_addmap(struct file *file, unsigned int cmd, } static int compat_drm_rmmap(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){ drm_map32_t __user *argp = (void __user *)arg; struct drm_map map;
- u32 handle;
- compat_uptr_t handle;
if (get_user(handle, &argp->handle)) return -EFAULT; @@ -249,61 +249,61 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd, } #endif -typedef struct drm_client32 {
- int idx; /* Which client desired? */
- int auth; /* Is client authenticated? */
- u32 pid; /* Process ID */
- u32 uid; /* User ID */
- u32 magic; /* Magic */
- u32 iocs; /* Ioctl count */
-} drm_client32_t; +typedef struct drm_client_compat {
- int idx; /* Which client desired? */
- int auth; /* Is client authenticated? */
- compat_ulong_t pid; /* Process ID */
- compat_ulong_t uid; /* User ID */
- compat_ulong_t magic; /* Magic */
- compat_ulong_t iocs; /* Ioctl count */
+} drm_client_compat_t; static int compat_drm_getclient(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){
- drm_client32_t c32;
- drm_client32_t __user *argp = (void __user *)arg;
- drm_client_compat_t c_compat;
- drm_client_compat_t __user *argp = (void __user *)arg;
This is rather strange as we're also changing the type of arg to user_uintptr_t, which normally means the user pointer conversion has already been done. It makes more sense to either 1. do the compat_ptr() conversion at the top-level (drm_compat_ioctl()) and change the prototype of these functions to use user_uintptr_t, or 2. not change their prototype and get each to call compat_ptr() themselves.
Kevin
struct drm_client client; int err;
- if (copy_from_user(&c32, argp, sizeof(c32)))
- if (copy_from_user(&c_compat, argp, sizeof(c_compat))) return -EFAULT;
memset(&client, 0, sizeof(client));
- client.idx = c32.idx;
- client.idx = c_compat.idx;
err = drm_ioctl_kernel(file, drm_getclient, &client, 0); if (err) return err;
- c32.idx = client.idx;
- c32.auth = client.auth;
- c32.pid = client.pid;
- c32.uid = client.uid;
- c32.magic = client.magic;
- c32.iocs = client.iocs;
- c_compat.idx = client.idx;
- c_compat.auth = client.auth;
- c_compat.pid = client.pid;
- c_compat.uid = client.uid;
- c_compat.magic = client.magic;
- c_compat.iocs = client.iocs;
- if (copy_to_user(argp, &c32, sizeof(c32)))
- if (copy_to_user(argp, &c_compat, sizeof(c_compat))) return -EFAULT; return 0;
} -typedef struct drm_stats32 {
- u32 count;
+typedef struct drm_stats_compat {
- compat_ulong_t count; struct {
u32 value;
compat_ulong_t value;-} drm_stats32_t; +} drm_stats_compat_t; static int compat_drm_getstats(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){
- drm_stats32_t __user *argp = (void __user *)arg;
- drm_stats_compat_t __user *argp = (void __user *)arg;
/* getstats is defunct, just clear */
- if (clear_user(argp, sizeof(drm_stats32_t)))
- if (clear_user(argp, sizeof(drm_stats_compat_t))) return -EFAULT; return 0;
} @@ -820,47 +820,47 @@ static int compat_drm_update_draw(struct file *file, unsigned int cmd, } #endif -struct drm_wait_vblank_request32 { +struct drm_wait_vblank_request_compat { enum drm_vblank_seq_type type; unsigned int sequence;
- u32 signal;
- compat_ulong_t signal;
}; -struct drm_wait_vblank_reply32 { +struct drm_wait_vblank_reply_compat { enum drm_vblank_seq_type type; unsigned int sequence; s32 tval_sec; s32 tval_usec; }; -typedef union drm_wait_vblank32 {
- struct drm_wait_vblank_request32 request;
- struct drm_wait_vblank_reply32 reply;
-} drm_wait_vblank32_t; +typedef union drm_wait_vblank_compat {
- struct drm_wait_vblank_request_compat request;
- struct drm_wait_vblank_reply_compat reply;
+} drm_wait_vblank_compat_t; static int compat_drm_wait_vblank(struct file *file, unsigned int cmd,
unsigned long arg)
user_uintptr_t arg){
- drm_wait_vblank32_t __user *argp = (void __user *)arg;
- drm_wait_vblank32_t req32;
- drm_wait_vblank_compat_t __user *argp = (void __user *)arg;
- drm_wait_vblank_compat_t req_compat; union drm_wait_vblank req; int err;
- if (copy_from_user(&req32, argp, sizeof(req32)))
- if (copy_from_user(&req_compat, argp, sizeof(req_compat))) return -EFAULT;
memset(&req, 0, sizeof(req));
- req.request.type = req32.request.type;
- req.request.sequence = req32.request.sequence;
- req.request.signal = req32.request.signal;
- req.request.type = req_compat.request.type;
- req.request.sequence = req_compat.request.sequence;
- req.request.signal = req_compat.request.signal; err = drm_ioctl_kernel(file, drm_wait_vblank_ioctl, &req, DRM_UNLOCKED);
- req32.reply.type = req.reply.type;
- req32.reply.sequence = req.reply.sequence;
- req32.reply.tval_sec = req.reply.tval_sec;
- req32.reply.tval_usec = req.reply.tval_usec;
- if (copy_to_user(argp, &req32, sizeof(req32)))
- req_compat.reply.type = req.reply.type;
- req_compat.reply.sequence = req.reply.sequence;
- req_compat.reply.tval_sec = req.reply.tval_sec;
- req_compat.reply.tval_usec = req.reply.tval_usec;
- if (copy_to_user(argp, &req_compat, sizeof(req_compat))) return -EFAULT;
return err; @@ -911,15 +911,16 @@ static struct { drm_ioctl_compat_t *fn; char *name; } drm_compat_ioctls[] = { +#define DRM_IOCTL_COMPAT_DEF(n, f) [DRM_IOCTL_NR(n##_COMPAT)] = {.fn = f, .name = #n} #define DRM_IOCTL32_DEF(n, f) [DRM_IOCTL_NR(n##32)] = {.fn = f, .name = #n}
- DRM_IOCTL32_DEF(DRM_IOCTL_VERSION, compat_drm_version),
- DRM_IOCTL32_DEF(DRM_IOCTL_GET_UNIQUE, compat_drm_getunique),
- DRM_IOCTL_COMPAT_DEF(DRM_IOCTL_VERSION, compat_drm_version),
- DRM_IOCTL_COMPAT_DEF(DRM_IOCTL_GET_UNIQUE, compat_drm_getunique),
#if IS_ENABLED(CONFIG_DRM_LEGACY) DRM_IOCTL32_DEF(DRM_IOCTL_GET_MAP, compat_drm_getmap), #endif
- DRM_IOCTL32_DEF(DRM_IOCTL_GET_CLIENT, compat_drm_getclient),
- DRM_IOCTL32_DEF(DRM_IOCTL_GET_STATS, compat_drm_getstats),
- DRM_IOCTL32_DEF(DRM_IOCTL_SET_UNIQUE, compat_drm_setunique),
- DRM_IOCTL_COMPAT_DEF(DRM_IOCTL_GET_CLIENT, compat_drm_getclient),
- DRM_IOCTL_COMPAT_DEF(DRM_IOCTL_GET_STATS, compat_drm_getstats),
- DRM_IOCTL_COMPAT_DEF(DRM_IOCTL_SET_UNIQUE, compat_drm_setunique),
#if IS_ENABLED(CONFIG_DRM_LEGACY) DRM_IOCTL32_DEF(DRM_IOCTL_ADD_MAP, compat_drm_addmap), DRM_IOCTL32_DEF(DRM_IOCTL_ADD_BUFS, compat_drm_addbufs), @@ -948,7 +949,7 @@ static struct { #if defined(CONFIG_X86) || defined(CONFIG_IA64) DRM_IOCTL32_DEF(DRM_IOCTL_UPDATE_DRAW, compat_drm_update_draw), #endif
- DRM_IOCTL32_DEF(DRM_IOCTL_WAIT_VBLANK, compat_drm_wait_vblank),
- DRM_IOCTL_COMPAT_DEF(DRM_IOCTL_WAIT_VBLANK, compat_drm_wait_vblank),
#if defined(CONFIG_X86) || defined(CONFIG_IA64) DRM_IOCTL32_DEF(DRM_IOCTL_MODE_ADDFB2, compat_drm_mode_addfb2), #endif diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h index 26d5c31aefa9..a7fd12d09268 100644 --- a/include/drm/drm_ioctl.h +++ b/include/drm/drm_ioctl.h @@ -65,7 +65,7 @@ typedef int drm_ioctl_t(struct drm_device *dev, void *data,
- structures and hence never need this.
*/ typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
unsigned long arg);
user_uintptr_t arg);#define DRM_IOCTL_NR(n) _IOC_NR(n) #define DRM_IOCTL_TYPE(n) _IOC_TYPE(n)