On 2/7/23 11:05, Kevin Brodsky wrote:

On 06/02/2023 11:46, carsten.haitzler@foss.arm.com wrote:

...

From: Carsten Haitzler carsten.haitzler@arm.com

DRM compat code totally assumes compat is 32bit. This is not always 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 | 55 +++++++++++++++++++------------------ include/drm/drm_ioctl.h | 2 +- 2 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioc32.c b/drivers/gpu/drm/drm_ioc32.c index 5d82891c3222..a642754da108 100644 --- a/drivers/gpu/drm/drm_ioc32.c +++ b/drivers/gpu/drm/drm_ioc32.c @@ -76,20 +76,20 @@ #define DRM_IOCTL_MODE_ADDFB232 DRM_IOWR(0xb8, drm_mode_fb_cmd232_t) -typedef struct drm_version_32 { +typedef struct drm_version32 {

Spurious change (went unnoticed as the struct is always referred to by its typedef).

gah! - yeah. missed that. this one doesn't follow the pattern of everything else...

...

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 */

• 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_version32_t;

static int compat_drm_version(struct file *file, unsigned int cmd,

• 	      unsigned long arg)
  

• 	      user_uintptr_t arg)
  

  { drm_version32_t v32; struct drm_version v;

@@ -125,12 +125,12 @@ static int compat_drm_version(struct file *file, unsigned int cmd, } typedef struct drm_unique32 {

• u32 unique_len; /* Length of unique */
• u32 unique; /* Unique name for driver instantiation */

• compat_size_t unique_len; /* Length of unique */
• compat_uptr_t unique; /* Unique name for driver instantiation */ } drm_unique32_t;

static int compat_drm_getunique(struct file *file, unsigned int cmd,

• 		unsigned long arg)
  

• 		user_uintptr_t arg)
  

  { drm_unique32_t uq32; struct drm_unique uq;

@@ -157,7 +157,7 @@ static int compat_drm_getunique(struct file *file, unsigned int cmd, } static int compat_drm_setunique(struct file *file, unsigned int cmd,

• 		unsigned long arg)
  

• 		user_uintptr_t arg)
  

  { /* it's dead */ return -EINVAL;

@@ -250,16 +250,16 @@ 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 */

• 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_client32_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;

@@ -290,15 +290,15 @@ static int compat_drm_getclient(struct file *file, unsigned int cmd, } typedef struct drm_stats32 {

• u32 count;

• compat_ulong_t count; struct {

• u32 value;
  

• compat_ulong_t value;
  

  enum drm_stat_type type; } data[15]; } drm_stats32_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;

@@ -823,7 +823,7 @@ static int compat_drm_update_draw(struct file *file, unsigned int cmd, struct drm_wait_vblank_request32 { enum drm_vblank_seq_type type; unsigned int sequence;

• u32 signal;

• compat_ulong_t signal; };

struct drm_wait_vblank_reply32 {

It looks like this one needs some conversion too, as tval_{,u}sec are represented as long in native but currently s32 here.

how did i forget this? and not see it as a problem either... i literally have code that uses vblank events for animation timing that i wrote that i have actually tested against this that should have tripped over...

oh that's right! i just checked my git log. i disabled the code that used the device timestamp because that timestamp is not actually useful as it's the timestamp of the top-left pixel but the interrupt can fire BEFORE that pixel is scanned out and thus it's possible to get timestamps from the device that are in the future... which really messes everything up when time jumps forward then soon after in the next loop iteration if its picking up the next clock time from the system clock not the vblank clock it jumps back in time if the system is fast enough to do 2 loop iterations between vblank interrupt and the first pixel scanout time... gah... i forgot i changed that so it's not picking up the problem. got to love timeline warping issues that happen only in some cases that take a while to figure out. :)

Kevin

...

@@ -839,7 +839,7 @@ typedef union drm_wait_vblank32 { } drm_wait_vblank32_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;

@@ -974,24 +974,25 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) struct drm_file *file_priv = filp->private_data; drm_ioctl_compat_t *fn; int ret;

• user_uintptr_t argp = (user_uintptr_t)compat_ptr(arg);

/* Assume that ioctls without an explicit compat routine will just * work. This may not always be a good assumption, but it's better * than always failing. */ if (nr >= ARRAY_SIZE(drm_compat_ioctls))

• return drm_ioctl(filp, cmd, arg);
  

• return drm_ioctl(filp, cmd, argp);
  

fn = drm_compat_ioctls[nr].fn; if (!fn)

• return drm_ioctl(filp, cmd, arg);
  

• return drm_ioctl(filp, cmd, argp);
  

DRM_DEBUG("comm="%s", pid=%d, dev=0x%lx, auth=%d, %s\n", current->comm, task_pid_nr(current), (long)old_encode_dev(file_priv->minor->kdev->devt), file_priv->authenticated, drm_compat_ioctls[nr].name);

• ret = (*fn)(filp, cmd, arg);

• ret = (*fn)(filp, cmd, argp); if (ret) DRM_DEBUG("ret = %d\n", ret); return ret;

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)