* BUG: unable to handle kernel NULL pointer dereference (drm_getunique) @ 2008-06-04 23:03 Sitsofe Wheeler 2008-06-04 23:42 ` Johannes Weiner 2008-06-05 0:54 ` Johannes Weiner 0 siblings, 2 replies; 7+ messages in thread From: Sitsofe Wheeler @ 2008-06-04 23:03 UTC (permalink / raw) To: linux-kernel While flipping back and forth between the vts/Xorg and doing iperf test over the wifi connection the following error appeared in next-20080604. [ 4305.767435] BUG: unable to handle kernel NULL pointer dereference at 00000000 [ 4305.767452] IP: [<c02511d4>] drm_getunique+0xc/0x30 [ 4305.767466] *pde = 00000000 [ 4305.767474] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC [ 4305.767483] last sysfs file: /sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map [ 4305.767492] Modules linked in: [ 4305.767498] [ 4305.767503] Pid: 8373, comm: unix2_chkpwd Tainted: G W (2.6.26-rc4-next-20080604skw #177) [ 4305.767513] EIP: 0060:[<c02511d4>] EFLAGS: 00210246 CPU: 0 [ 4305.767521] EIP is at drm_getunique+0xc/0x30 [ 4305.767527] EAX: f7d8ea70 EBX: 00000000 ECX: 00000028 EDX: 00000000 [ 4305.767535] ESI: f7d8ea70 EDI: 00005401 EBP: ed383f34 ESP: ed383f2c [ 4305.767543] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 [ 4305.767550] Process unix2_chkpwd (pid: 8373, ti=ed382000 task=f6efdee0 task.ti=ed382000) [ 4305.767558] Stack: 00000000 c04940a4 ed383f58 c0250637 f1e03f50 f7d8ea70 f7d8eaf0 c02511c8 [ 4305.767574] c04944c4 d603b200 bfda0500 ed383f74 c017ebba bfda0500 00005401 d603b200 [ 4305.767589] d603b200 bfda0500 ed383f98 c017ee0a 00000001 ffffffea f1c0c100 ed383fb0 [ 4305.767605] Call Trace: [ 4305.767611] [<c0250637>] ? drm_ioctl+0x1b0/0x225 [ 4305.767622] [<c02511c8>] ? drm_getunique+0x0/0x30 [ 4305.767632] [<c017ebba>] ? vfs_ioctl+0x4e/0x67 [ 4305.767643] [<c017ee0a>] ? do_vfs_ioctl+0x237/0x245 [ 4305.767652] [<c017ee44>] ? sys_ioctl+0x2c/0x48 [ 4305.767661] [<c0102c35>] ? sysenter_past_esp+0x6a/0xa5 [ 4305.767672] ======================= [ 4305.767678] Code: 0f 18 01 90 8d 42 18 39 d8 75 c0 8b 45 ec e8 fa e4 11 00 b8 ea ff ff ff 59 5b 5b 5e 5f 5d c3 55 89 e5 56 89 c6 53 8b 48 04 89 d3 <39> 0a 72 13 8b 42 04 8b 16 e8 a1 df f9 ff ba f2 ff ff ff 85 c0 [ 4305.767747] EIP: [<c02511d4>] drm_getunique+0xc/0x30 SS:ESP 0068:ed383f2c [ 4305.770317] ---[ end trace 032f7186de7f2687 ]--- Config is here: http://sucs.org/~sits/test/config-20080604.gz -- Sitsofe | http://sucs.org/~sits/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference (drm_getunique) 2008-06-04 23:03 BUG: unable to handle kernel NULL pointer dereference (drm_getunique) Sitsofe Wheeler @ 2008-06-04 23:42 ` Johannes Weiner 2008-06-05 7:08 ` Sitsofe Wheeler 2008-06-05 0:54 ` Johannes Weiner 1 sibling, 1 reply; 7+ messages in thread From: Johannes Weiner @ 2008-06-04 23:42 UTC (permalink / raw) To: Sitsofe Wheeler; +Cc: linux-kernel Hi, Sitsofe Wheeler <sitsofe@yahoo.com> writes: > While flipping back and forth between the vts/Xorg and doing iperf test > over the wifi connection the following error appeared in next-20080604. > > [ 4305.767435] BUG: unable to handle kernel NULL pointer dereference at 00000000 > [ 4305.767452] IP: [<c02511d4>] drm_getunique+0xc/0x30 > [ 4305.767466] *pde = 00000000 > [ 4305.767474] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC > [ 4305.767483] last sysfs file: /sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map > [ 4305.767492] Modules linked in: > [ 4305.767498] > [ 4305.767503] Pid: 8373, comm: unix2_chkpwd Tainted: G W (2.6.26-rc4-next-20080604skw #177) There was a warning before that oops already, could you post that too? > [ 4305.767513] EIP: 0060:[<c02511d4>] EFLAGS: 00210246 CPU: 0 > [ 4305.767521] EIP is at drm_getunique+0xc/0x30 > [ 4305.767527] EAX: f7d8ea70 EBX: 00000000 ECX: 00000028 EDX: 00000000 > [ 4305.767535] ESI: f7d8ea70 EDI: 00005401 EBP: ed383f34 ESP: ed383f2c > [ 4305.767543] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 > [ 4305.767550] Process unix2_chkpwd (pid: 8373, ti=ed382000 task=f6efdee0 task.ti=ed382000) > [ 4305.767558] Stack: 00000000 c04940a4 ed383f58 c0250637 f1e03f50 f7d8ea70 f7d8eaf0 c02511c8 > [ 4305.767574] c04944c4 d603b200 bfda0500 ed383f74 c017ebba bfda0500 00005401 d603b200 > [ 4305.767589] d603b200 bfda0500 ed383f98 c017ee0a 00000001 ffffffea f1c0c100 ed383fb0 > [ 4305.767605] Call Trace: > [ 4305.767611] [<c0250637>] ? drm_ioctl+0x1b0/0x225 > [ 4305.767622] [<c02511c8>] ? drm_getunique+0x0/0x30 > [ 4305.767632] [<c017ebba>] ? vfs_ioctl+0x4e/0x67 > [ 4305.767643] [<c017ee0a>] ? do_vfs_ioctl+0x237/0x245 > [ 4305.767652] [<c017ee44>] ? sys_ioctl+0x2c/0x48 > [ 4305.767661] [<c0102c35>] ? sysenter_past_esp+0x6a/0xa5 > [ 4305.767672] ======================= > [ 4305.767678] Code: 0f 18 01 90 8d 42 18 39 d8 75 c0 8b 45 ec e8 fa e4 11 00 b8 ea ff ff ff 59 5b 5b 5e 5f 5d c3 55 89 e5 56 89 c6 53 8b 48 04 89 d3 <39> 0a 72 13 8b 42 04 8b 16 e8 a1 df f9 ff ba f2 ff ff ff 85 c0 > [ 4305.767747] EIP: [<c02511d4>] drm_getunique+0xc/0x30 SS:ESP 0068:ed383f2c > [ 4305.770317] ---[ end trace 032f7186de7f2687 ]--- Hannes ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference (drm_getunique) 2008-06-04 23:42 ` Johannes Weiner @ 2008-06-05 7:08 ` Sitsofe Wheeler 0 siblings, 0 replies; 7+ messages in thread From: Sitsofe Wheeler @ 2008-06-05 7:08 UTC (permalink / raw) To: linux-kernel Hello, Johannes Weiner wrote: > Sitsofe Wheeler <sitsofe@yahoo.com> writes: > >> [ 4305.767503] Pid: 8373, comm: unix2_chkpwd Tainted: G W >> [ (2.6.26-rc4-next-20080604skw #177) > > There was a warning before that oops already, could you post that too? The warning is: [ 42.949862] ------------[ cut here ]------------ [ 42.949873] WARNING: at kernel/lockdep.c:2680 check_flags+0x8a/0x12d() [ 42.949880] Modules linked in: [ 42.949887] Pid: 5, comm: watchdog/0 Not tainted 2.6.26-rc4-next-20080604skw #178 [ 42.949898] [<c01226f0>] warn_on_slowpath+0x41/0x6a [ 42.949917] [<c013c584>] ? trace_hardirqs_off+0xb/0xd [ 42.949935] [<c013e3b4>] ? trace_hardirqs_on+0xb/0xd [ 42.951740] [<c013e36d>] ? trace_hardirqs_on_caller+0xe8/0x124 [ 42.951759] [<c013e3b4>] ? trace_hardirqs_on+0xb/0xd [ 42.951778] [<c011c8f7>] ? hrtick_set+0xce/0xd6 [ 42.951799] [<c013c1d2>] check_flags+0x8a/0x12d [ 42.951816] [<c013f2f8>] lock_acquire+0x3b/0x89 [ 42.951830] [<c0370c0a>] _read_lock+0x1c/0x49 [ 42.951848] [<c01531fe>] ? watchdog+0x97/0x1a9 [ 42.951867] [<c0153167>] ? watchdog+0x0/0x1a9 [ 42.951883] [<c01531fe>] watchdog+0x97/0x1a9 [ 42.951900] [<c0153167>] ? watchdog+0x0/0x1a9 [ 42.951916] [<c01329a0>] kthread+0x3b/0x63 [ 42.951933] [<c0132965>] ? kthread+0x0/0x63 [ 42.951952] [<c01038ab>] kernel_thread_helper+0x7/0x10 [ 42.951969] ======================= [ 42.951976] ---[ end trace 199a1fe68fc13dfd ]--- [ 42.951983] possible reason: unannotated irqs-on. [ 42.951991] irq event stamp: 18 [ 42.951998] hardirqs last enabled at (17): [<c013e3b4>] trace_hardirqs_on+0xb/0xd [ 42.952018] hardirqs last disabled at (18): [<c013c584>] trace_hardirqs_off+0xb/0xd [ 42.952043] softirqs last enabled at (0): [<c0120e1b>] copy_process+0x2dd/0xf9a [ 42.952062] softirqs last disabled at (0): [<00000000>] 0x0 I've mentioned it before ( http://lkml.org/lkml/2008/5/29/502 ) and even bisected it ( http://lkml.org/lkml/2008/6/4/313 ). I guess the problem is that everyone is busy and it's hard to tell if people didn't see it, if it's just not important. Perhaps there needs to be a rule that lockdep warnings always go into bugzilla (or that if you are willing to chase an issue for a minimum of six months it should be bugzillad...). Thanks for your work looking at the real bug though! -- Sitsofe | http://sucs.org/~sits/ ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference (drm_getunique) 2008-06-04 23:03 BUG: unable to handle kernel NULL pointer dereference (drm_getunique) Sitsofe Wheeler 2008-06-04 23:42 ` Johannes Weiner @ 2008-06-05 0:54 ` Johannes Weiner 2008-06-05 2:21 ` Johannes Weiner 1 sibling, 1 reply; 7+ messages in thread From: Johannes Weiner @ 2008-06-05 0:54 UTC (permalink / raw) To: Sitsofe Wheeler; +Cc: linux-kernel, David Airlie Hi, Sitsofe Wheeler <sitsofe@yahoo.com> writes: > While flipping back and forth between the vts/Xorg and doing iperf test > over the wifi connection the following error appeared in next-20080604. > > [ 4305.767435] BUG: unable to handle kernel NULL pointer dereference at 00000000 > [ 4305.767452] IP: [<c02511d4>] drm_getunique+0xc/0x30 > [ 4305.767466] *pde = 00000000 > [ 4305.767474] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC > [ 4305.767483] last sysfs file: /sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map > [ 4305.767492] Modules linked in: > [ 4305.767498] > [ 4305.767503] Pid: 8373, comm: unix2_chkpwd Tainted: G W (2.6.26-rc4-next-20080604skw #177) > [ 4305.767513] EIP: 0060:[<c02511d4>] EFLAGS: 00210246 CPU: 0 > [ 4305.767521] EIP is at drm_getunique+0xc/0x30 > [ 4305.767527] EAX: f7d8ea70 EBX: 00000000 ECX: 00000028 EDX: 00000000 > [ 4305.767535] ESI: f7d8ea70 EDI: 00005401 EBP: ed383f34 ESP: ed383f2c > [ 4305.767543] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 > [ 4305.767550] Process unix2_chkpwd (pid: 8373, ti=ed382000 task=f6efdee0 task.ti=ed382000) > [ 4305.767558] Stack: 00000000 c04940a4 ed383f58 c0250637 f1e03f50 f7d8ea70 f7d8eaf0 c02511c8 > [ 4305.767574] c04944c4 d603b200 bfda0500 ed383f74 c017ebba bfda0500 00005401 d603b200 > [ 4305.767589] d603b200 bfda0500 ed383f98 c017ee0a 00000001 ffffffea f1c0c100 ed383fb0 > [ 4305.767605] Call Trace: > [ 4305.767611] [<c0250637>] ? drm_ioctl+0x1b0/0x225 > [ 4305.767622] [<c02511c8>] ? drm_getunique+0x0/0x30 > [ 4305.767632] [<c017ebba>] ? vfs_ioctl+0x4e/0x67 > [ 4305.767643] [<c017ee0a>] ? do_vfs_ioctl+0x237/0x245 > [ 4305.767652] [<c017ee44>] ? sys_ioctl+0x2c/0x48 > [ 4305.767661] [<c0102c35>] ? sysenter_past_esp+0x6a/0xa5 Hm, in drm_getunique, dev is not NULL as it was already dereferenced in drm_ioctl. file_priv is not used at all. So only data is left. data is kdata in drm_ioctl and only NULL if the ioctl request is neither input nor output. I have not found a check on cmd in the callpath that would filter out malformed requests. So the user would be allowed to pass in a valid request number with wrong flags here, correct? Hannes --- drm: check ioctl direction before dispatching The userland might specify a valid ioctl request number with a wrong direction. Precheck the direction of the dispatcher function beforehand because we prepare arguments based on this parameter and the dispatched function relies on them being valid. Signed-off-by: Johannes Weiner <hannes@saeurebad.de> CC: David Airlie <airlied@redhat.com> --- Compile-time tested only! diff --git a/drivers/char/drm/drm_drv.c b/drivers/char/drm/drm_drv.c index fc54140..1a27d04 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; + /* Make sure the direction is correct */ + if (_IOC_DIR(ioctl->cmd) != _IOC_DIR(cmd)) + goto err_i1; + func = ioctl->func; /* is there a local override? */ if ((nr == DRM_IOCTL_NR(DRM_IOCTL_DMA)) && dev->driver->dma_ioctl) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference (drm_getunique) 2008-06-05 0:54 ` Johannes Weiner @ 2008-06-05 2:21 ` Johannes Weiner 2008-06-05 3:05 ` Dave Airlie 0 siblings, 1 reply; 7+ messages in thread From: Johannes Weiner @ 2008-06-05 2:21 UTC (permalink / raw) To: Sitsofe Wheeler; +Cc: linux-kernel, David Airlie Hi, Johannes Weiner <hannes@saeurebad.de> writes: > Hi, > > Sitsofe Wheeler <sitsofe@yahoo.com> writes: > >> While flipping back and forth between the vts/Xorg and doing iperf test >> over the wifi connection the following error appeared in next-20080604. >> >> [ 4305.767435] BUG: unable to handle kernel NULL pointer dereference at 00000000 >> [ 4305.767452] IP: [<c02511d4>] drm_getunique+0xc/0x30 >> [ 4305.767466] *pde = 00000000 >> [ 4305.767474] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC >> [ 4305.767483] last sysfs file: /sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map >> [ 4305.767492] Modules linked in: >> [ 4305.767498] >> [ 4305.767503] Pid: 8373, comm: unix2_chkpwd Tainted: G W (2.6.26-rc4-next-20080604skw #177) >> [ 4305.767513] EIP: 0060:[<c02511d4>] EFLAGS: 00210246 CPU: 0 >> [ 4305.767521] EIP is at drm_getunique+0xc/0x30 >> [ 4305.767527] EAX: f7d8ea70 EBX: 00000000 ECX: 00000028 EDX: 00000000 >> [ 4305.767535] ESI: f7d8ea70 EDI: 00005401 EBP: ed383f34 ESP: ed383f2c >> [ 4305.767543] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 >> [ 4305.767550] Process unix2_chkpwd (pid: 8373, ti=ed382000 task=f6efdee0 task.ti=ed382000) >> [ 4305.767558] Stack: 00000000 c04940a4 ed383f58 c0250637 f1e03f50 f7d8ea70 f7d8eaf0 c02511c8 >> [ 4305.767574] c04944c4 d603b200 bfda0500 ed383f74 c017ebba bfda0500 00005401 d603b200 >> [ 4305.767589] d603b200 bfda0500 ed383f98 c017ee0a 00000001 ffffffea f1c0c100 ed383fb0 >> [ 4305.767605] Call Trace: >> [ 4305.767611] [<c0250637>] ? drm_ioctl+0x1b0/0x225 >> [ 4305.767622] [<c02511c8>] ? drm_getunique+0x0/0x30 >> [ 4305.767632] [<c017ebba>] ? vfs_ioctl+0x4e/0x67 >> [ 4305.767643] [<c017ee0a>] ? do_vfs_ioctl+0x237/0x245 >> [ 4305.767652] [<c017ee44>] ? sys_ioctl+0x2c/0x48 >> [ 4305.767661] [<c0102c35>] ? sysenter_past_esp+0x6a/0xa5 > > Hm, in drm_getunique, dev is not NULL as it was already dereferenced in > drm_ioctl. file_priv is not used at all. So only data is left. > > data is kdata in drm_ioctl and only NULL if the ioctl request is neither > input nor output. > > I have not found a check on cmd in the callpath that would filter out > malformed requests. So the user would be allowed to pass in a valid > request number with wrong flags here, correct? > > Hannes > > --- > drm: check ioctl direction before dispatching > > The userland might specify a valid ioctl request number with a wrong > direction. Precheck the direction of the dispatcher function beforehand > because we prepare arguments based on this parameter and the dispatched > function relies on them being valid. > > Signed-off-by: Johannes Weiner <hannes@saeurebad.de> > CC: David Airlie <airlied@redhat.com> > --- > > Compile-time tested only! 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) ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference (drm_getunique) 2008-06-05 2:21 ` Johannes Weiner @ 2008-06-05 3:05 ` Dave Airlie 2008-06-05 18:04 ` Johannes Weiner 0 siblings, 1 reply; 7+ messages in thread From: Dave Airlie @ 2008-06-05 3:05 UTC (permalink / raw) To: Johannes Weiner; +Cc: Sitsofe Wheeler, linux-kernel On Thu, 2008-06-05 at 04:21 +0200, Johannes Weiner wrote: > Hi, > > Johannes Weiner <hannes@saeurebad.de> writes: > > > Hi, > > > > Sitsofe Wheeler <sitsofe@yahoo.com> writes: > > > >> While flipping back and forth between the vts/Xorg and doing iperf test > >> over the wifi connection the following error appeared in next-20080604. > >> > >> [ 4305.767435] BUG: unable to handle kernel NULL pointer dereference at 00000000 > >> [ 4305.767452] IP: [<c02511d4>] drm_getunique+0xc/0x30 > >> [ 4305.767466] *pde = 00000000 > >> [ 4305.767474] Oops: 0000 [#1] SMP DEBUG_PAGEALLOC > >> [ 4305.767483] last sysfs file: /sys/devices/system/cpu/cpu1/cache/index2/shared_cpu_map > >> [ 4305.767492] Modules linked in: > >> [ 4305.767498] > >> [ 4305.767503] Pid: 8373, comm: unix2_chkpwd Tainted: G W (2.6.26-rc4-next-20080604skw #177) > >> [ 4305.767513] EIP: 0060:[<c02511d4>] EFLAGS: 00210246 CPU: 0 > >> [ 4305.767521] EIP is at drm_getunique+0xc/0x30 > >> [ 4305.767527] EAX: f7d8ea70 EBX: 00000000 ECX: 00000028 EDX: 00000000 > >> [ 4305.767535] ESI: f7d8ea70 EDI: 00005401 EBP: ed383f34 ESP: ed383f2c > >> [ 4305.767543] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 > >> [ 4305.767550] Process unix2_chkpwd (pid: 8373, ti=ed382000 task=f6efdee0 task.ti=ed382000) > >> [ 4305.767558] Stack: 00000000 c04940a4 ed383f58 c0250637 f1e03f50 f7d8ea70 f7d8eaf0 c02511c8 > >> [ 4305.767574] c04944c4 d603b200 bfda0500 ed383f74 c017ebba bfda0500 00005401 d603b200 > >> [ 4305.767589] d603b200 bfda0500 ed383f98 c017ee0a 00000001 ffffffea f1c0c100 ed383fb0 > >> [ 4305.767605] Call Trace: > >> [ 4305.767611] [<c0250637>] ? drm_ioctl+0x1b0/0x225 > >> [ 4305.767622] [<c02511c8>] ? drm_getunique+0x0/0x30 > >> [ 4305.767632] [<c017ebba>] ? vfs_ioctl+0x4e/0x67 > >> [ 4305.767643] [<c017ee0a>] ? do_vfs_ioctl+0x237/0x245 > >> [ 4305.767652] [<c017ee44>] ? sys_ioctl+0x2c/0x48 > >> [ 4305.767661] [<c0102c35>] ? sysenter_past_esp+0x6a/0xa5 > > > > Hm, in drm_getunique, dev is not NULL as it was already dereferenced in > > drm_ioctl. file_priv is not used at all. So only data is left. > > > > data is kdata in drm_ioctl and only NULL if the ioctl request is neither > > input nor output. > > > > I have not found a check on cmd in the callpath that would filter out > > malformed requests. So the user would be allowed to pass in a valid > > request number with wrong flags here, correct? > > > > Hannes > > > > --- > > drm: check ioctl direction before dispatching > > > > The userland might specify a valid ioctl request number with a wrong > > direction. Precheck the direction of the dispatcher function beforehand > > because we prepare arguments based on this parameter and the dispatched > > function relies on them being valid. > > > > Signed-off-by: Johannes Weiner <hannes@saeurebad.de> > > CC: David Airlie <airlied@redhat.com> > > --- > > > > Compile-time tested only! > > 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. > This patch will break some other parts of the DRM interface, I'll try and come up with one that is less likely do stop all the drivers working. The problem is there are binaries out there with bad userspace ioctl bits.. What I'll change it to do is at least allocate the size from the kernel definition not what the user passed in. The big problem we have with malformed requests is that userspace asks for one thing and expects another, so I should key the userspace from the ioctl nr and use the kernel side definitions to decide the other bits. Clearly the drm code isn't doing that now. Thanks, Dave. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: BUG: unable to handle kernel NULL pointer dereference (drm_getunique) 2008-06-05 3:05 ` Dave Airlie @ 2008-06-05 18:04 ` Johannes Weiner 0 siblings, 0 replies; 7+ messages in thread From: Johannes Weiner @ 2008-06-05 18:04 UTC (permalink / raw) To: Dave Airlie; +Cc: Sitsofe Wheeler, linux-kernel Hi, Dave Airlie <airlied@redhat.com> 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. >> > > > This patch will break some other parts of the DRM interface, I'll try > and come up with one that is less likely do stop all the drivers > working. We do not want that to happen, of course :) > The problem is there are binaries out there with bad userspace ioctl > bits.. > > What I'll change it to do is at least allocate the size from the kernel > definition not what the user passed in. > > The big problem we have with malformed requests is that userspace asks > for one thing and expects another, so I should key the userspace from > the ioctl nr and use the kernel side definitions to decide the other > bits. Clearly the drm code isn't doing that now. Hm, like this? diff --git a/drivers/char/drm/drm_drv.c b/drivers/char/drm/drm_drv.c index fc54140..019bf1f 100644 --- a/drivers/char/drm/drm_drv.c +++ b/drivers/char/drm/drm_drv.c @@ -475,6 +475,9 @@ int drm_ioctl(struct inode *inode, struct file *filp, else goto err_i1; + /* Do not trust userspace, use our own definition */ + cmd = ioctl->cmd; + func = ioctl->func; /* is there a local override? */ if ((nr == DRM_IOCTL_NR(DRM_IOCTL_DMA)) && dev->driver->dma_ioctl) ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2008-06-05 18:05 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-06-04 23:03 BUG: unable to handle kernel NULL pointer dereference (drm_getunique) Sitsofe Wheeler 2008-06-04 23:42 ` Johannes Weiner 2008-06-05 7:08 ` Sitsofe Wheeler 2008-06-05 0:54 ` Johannes Weiner 2008-06-05 2:21 ` Johannes Weiner 2008-06-05 3:05 ` Dave Airlie 2008-06-05 18:04 ` Johannes Weiner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox