Hi,
Johannes Weiner <hannes@saeurebad.de> writes:
Make that run-time tested, also. This does not just affect linux-next.
The following program oopses my box running Linus' current git:
#include <fcntl.h>
#include <unistd.h>
#include <sys/ioctl.h>
#include <drm/drm.h>
#define DRM_DFILE "/dev/dri/card0"
#define DRM_IOCTL_CAPUT _IO(DRM_IOCTL_BASE, 0x01)
int main(void)
{
int fd = open(DRM_DFILE, O_RDONLY);
if (fd < 0)
return 1;
ioctl(fd, DRM_IOCTL_CAPUT);
perror("ioctl");
close(fd);
return 0;
}
->
[ 714.581505] BUG: unable to handle kernel NULL pointer dereference at 00000000
[ 714.583459] IP: [<c027d491>] drm_getunique+0x11/0x40
[ 714.585460] *pde = 00000000
[ 714.587389] Oops: 0000 [#1] PREEMPT
[ 714.588271] Modules linked in: snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm snd_page_alloc
[ 714.588271]
[ 714.588271] Pid: 1339, comm: break-drm Not tainted (2.6.26-rc4-00232-gedeb280 #43)
[ 714.588271] EIP: 0060:[<c027d491>] EFLAGS: 00010282 CPU: 0
[ 714.588271] EIP is at drm_getunique+0x11/0x40
[ 714.588271] EAX: cf861000 EBX: 00000000 ECX: 00000000 EDX: 00000000
[ 714.588271] ESI: cf861000 EDI: cfa87700 EBP: 00000000 ESP: cecc3f20
[ 714.588271] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068
[ 714.588271] Process break-drm (pid: 1339, ti=cecc2000 task=cf9a6dc0 task.ti=cecc2000)
[ 714.588271] Stack: c027d480 cf861000 c027c592 ced53b7c b7fc00d0 c0159f3b ced53b7c 000000ce
[ 714.588271] 00000000 00000000 00006401 00000000 cf861024 c04730a0 ced33700 bf965788
[ 714.588271] 00006401 c017aea8 bf965788 ced33700 ced33700 00000003 cecc2000 c017b0fb
[ 714.588271] Call Trace:
[ 714.588271] [<c027d480>] drm_getunique+0x0/0x40
[ 714.588271] [<c027c592>] drm_ioctl+0x1a2/0x2e0
[ 714.588271] [<c0159f3b>] handle_mm_fault+0xeb/0x5e0
[ 714.588271] [<c017aea8>] vfs_ioctl+0x78/0x90
[ 714.588271] [<c017b0fb>] do_vfs_ioctl+0x23b/0x2c0
[ 714.588271] [<c016ce2a>] do_sys_open+0xba/0xe0
[ 714.588271] [<c017b1bd>] sys_ioctl+0x3d/0x70
[ 714.588271] [<c0103152>] syscall_call+0x7/0xb
[ 714.588271] =======================
[ 714.588271] Code: 8b 04 24 e8 e2 1b 12 00 31 c0 5b 5b 5e 5f 5d c3 8d 76 00 8d bc 27 00 00 00 00 83 ec 08 89 1c 24 89 d3 89 74 24 04 89 c6 8b 48 04 <39> 0a 73 11 89 0b 31 d2 8b 1c 24 89 d0 8b 74 24 04 83 c4 08 c3
[ 714.588271] EIP: [<c027d491>] drm_getunique+0x11/0x40 SS:ESP 0068:cecc3f20
[ 714.659745] ---[ end trace fd8339ca8c62cb63 ]---
[ 714.663840] [drm:drm_release] *ERROR* Device busy: 1 0
My patch fixes this, but here is a better one that checks the whole cmd
against the allowed one. Otherwise, it would be possible to trap the
kernel into allocating 2^15-1 bytes through kmalloc, too, at least on
this configuration here with _IOC_SIZEBITS == 14.
---
drm: sanity-check ioctl request
drm_ioctl looks up a dispatcher function for special ioctls but it does
not precheck the commands against the registered ones, allowing users to
pass bogus values and potentially oops the kernel, as happened to
Sitsofe Wheeler who reported this bug.
This patch checks the incoming ioctl command against the defined ones
before dispatching to the handler.
Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
Signed-off-by: Johannes Weiner <hannes@saeurebad.de>
CC: David Airlie <airlied@redhat.com>
---
diff --git a/drivers/char/drm/drm_drv.c b/drivers/char/drm/drm_drv.c
index fc54140..9731f2a 100644
--- a/drivers/char/drm/drm_drv.c
+++ b/drivers/char/drm/drm_drv.c
@@ -475,6 +475,10 @@ int drm_ioctl(struct inode *inode, struct file *filp,
else
goto err_i1;
+ /* user request sane? */
+ if (ioctl->cmd != cmd)
+ goto err_i1;
+
func = ioctl->func;
/* is there a local override? */
if ((nr == DRM_IOCTL_NR(DRM_IOCTL_DMA)) && dev->driver->dma_ioctl)
--
