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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <[email protected]>

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 <[email protected]>
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;
 }

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux