2005-10-31 20:35:35

by Blaisorblade

[permalink] [raw]

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

---

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

---

Attachments:

(No filename) (1.78 kB)
fix-drm-warn-x86-64 (1.78 kB)
Download all attachments

2005-10-31 22:21:54

by Dave Airlie

[permalink] [raw]

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

---

> 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

This is already fixed in the -mm tree from my -git archive.. I'll push it
to Linus this weekend hopefully..

> * If we _ever_ have more drm_device_t, the call to HandleId() would be
> racy. Right?

Potentially it might be alright, I suppose I could move the handle base
into the drm device, in practice we won't hit this yet, as the X server is
the only client that can do this stuff.. and it doesn't do threading..

Dave.

--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG