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/
Hi,
Sitsofe Wheeler <[email protected]> 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
Hi,
Sitsofe Wheeler <[email protected]> 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 <[email protected]>
CC: David Airlie <[email protected]>
---
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)
Hi,
Johannes Weiner <[email protected]> writes:
> Hi,
>
> Sitsofe Wheeler <[email protected]> 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 <[email protected]>
> CC: David Airlie <[email protected]>
> ---
>
> 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 <[email protected]>
Signed-off-by: Johannes Weiner <[email protected]>
CC: David Airlie <[email protected]>
---
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)
On Thu, 2008-06-05 at 04:21 +0200, Johannes Weiner wrote:
> Hi,
>
> Johannes Weiner <[email protected]> writes:
>
> > Hi,
> >
> > Sitsofe Wheeler <[email protected]> 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 <[email protected]>
> > CC: David Airlie <[email protected]>
> > ---
> >
> > 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.
Hello,
Johannes Weiner wrote:
> Sitsofe Wheeler <[email protected]> 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/
Hi,
Dave Airlie <[email protected]> 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)