[PATCH] DRM: 64-bit warning in compilation: wrong param size in DRM or harmless?

From: Blaisorblade
Date: Mon Oct 31 2005 - 15:35:31 EST


I got a warning and while going to fix it, I discovered some issues with the
code (including raciness).

While compiling 2.6.14 for x86_64, I got:

CC [M] drivers/char/drm/drm_bufs.o
/home/paolo/Admin/kernel/6/VCS/linux-2.6.14/drivers/char/drm/drm_bufs.c: In
function `drm_addmap_ioctl':
/home/paolo/Admin/kernel/6/VCS/linux-2.6.14/drivers/char/drm/drm_bufs.c:309:
warning: cast to pointer from integer of different size
/home/paolo/Admin/kernel/6/VCS/linux-2.6.14/drivers/char/drm/drm_bufs.c:309:
warning: cast to pointer from integer of different size
/home/paolo/Admin/kernel/6/VCS/linux-2.6.14/drivers/char/drm/drm_bufs.c:309:
warning: cast to pointer from integer of different size
/home/paolo/Admin/kernel/6/VCS/linux-2.6.14/drivers/char/drm/drm_bufs.c:309:
warning: cast to pointer from integer of different size

All these warnings are generated by:
if (put_user(maplist->user_token, &argp->handle))
return -EFAULT;

Given the decls:
drm_map_list_t *maplist;
drm_map_t __user *argp = (void __user *)arg;

typedef struct drm_map {
...
void *handle;
/**< User-space: "Handle" to pass to mmap()*/
/**< Kernel-space: kernel-virtual address */
...
} drm_map_t;

maplist->user_token is an unsigned int, instead.

It seems that even if handle is overloaded, the two roles are totally
different and never interchanged, but I'm unsure.

In this case, the warning is totally harmless, so the attached patch avoids
the warning and fixes everything (compile-tested).

BUT:

* If we _ever_ have more drm_device_t, the call to HandleId() would be racy.
Right?
--
Inform me of my mistakes, so I can keep imitating Homer Simpson's "Doh!".
Paolo Giarrusso, aka Blaisorblade (Skype ID "PaoloGiarrusso", ICQ 215621894)
http://www.user-mode-linux.org/~blaisorblade
DRM - Fix cast compile warning

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>

Current code, on x86-64, gives:
drivers/char/drm/drm_bufs.c:309: warning: cast to pointer from integer of different size

This results because ->handle is used by kernelspace as a pointer, and userspace
as 32-bit unique id. But this warning should be harmless as the ->handle member
should never be used in both its roles, though it could be instead - the id used
is actually very similar to a pointer, see HandleID().

Btw, if we _ever_ have more drm_device_t, the call to HandleId() would be racy.

Signed-off-by: Paolo 'Blaisorblade' Giarrusso <blaisorblade@xxxxxxxx>
Index: linux-2.6.git/drivers/char/drm/drm.h
===================================================================
--- linux-2.6.git.orig/drivers/char/drm/drm.h
+++ linux-2.6.git/drivers/char/drm/drm.h
@@ -245,7 +245,7 @@ typedef struct drm_map {
unsigned long size; /**< Requested physical size (bytes) */
drm_map_type_t type; /**< Type of memory to map */
drm_map_flags_t flags; /**< Flags */
- void *handle; /**< User-space: "Handle" to pass to mmap() */
+ void *handle; /**< User-space: 32-bit "handle" to pass to mmap() */
/**< Kernel-space: kernel-virtual address */
int mtrr; /**< MTRR slot used */
/* Private data */
Index: linux-2.6.git/drivers/char/drm/drm_bufs.c
===================================================================
--- linux-2.6.git.orig/drivers/char/drm/drm_bufs.c
+++ linux-2.6.git/drivers/char/drm/drm_bufs.c
@@ -306,7 +306,7 @@ int drm_addmap_ioctl(struct inode *inode

if (copy_to_user(argp, maplist->map, sizeof(drm_map_t)))
return -EFAULT;
- if (put_user(maplist->user_token, &argp->handle))
+ if (put_user((unsigned long) maplist->user_token, &argp->handle))
return -EFAULT;
return 0;
}