[Cluster-devel] [PATCH] checking NULL pointer in device_write of dlm-control

[Masatake YAMATO <yamato@redhat.com>]

Hi,

I found a way to let linux dereference NULL pointer
in gfs2-2.6-nmw/fs/dlm/user.c. 

If `device_write' method is called via "dlm-control", 
file->private_data is NULL. (See ctl_device_open() in 
user.c. ) Through proc->flags is read:

	if ((kbuf->cmd == DLM_USER_LOCK || kbuf->cmd == DLM_USER_UNLOCK) &&
	    test_bit(DLM_PROC_FLAGS_CLOSING, &proc->flags))
		return -EINVAL;

It causes following message on my Fedora 9 PC:

    BUG: unable to handle kernel NULL pointer dereference at 00000004
    IP: [<f8f555df>] :dlm:device_write+0xa5/0x432
    *pde = 7f11b067 
    Oops: 0000 [#2] SMP 
    Modules linked in: ...<snipped>

    Pid: 26899, comm: a.out Tainted: G      D  (2.6.25-14.fc9.i686 #1)
    EIP: 0060:[<f8f555df>] EFLAGS: 00210297 CPU: 1
    EIP is at device_write+0xa5/0x432 [dlm]
    EAX: f66ad200 EBX: f66ad280 ECX: 00000000 EDX: 00000006
    ESI: 00000064 EDI: 00000000 EBP: c8a45f70 ESP: c8a45f44
     DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
    Process a.out (pid: 26899, ti=c8a45000 task=c8a72000 task.ti=c8a45000)
    Stack: bfe90b34 f66ad280 00000000 c8a45f58 c04cc41c c8a45f70 c0482bd5 00000001 
	   def7a0c0 f8f5553a 00000064 c8a45f90 c04832bb c8a45f9c bfe90b34 c04817e7 
	   def7a0c0 fffffff7 080483a0 c8a45fb0 c04833f8 c8a45f9c 00000000 00000000 
    Call Trace:
     [<c04cc41c>] ? security_file_permission+0xf/0x11
     [<c0482bd5>] ? rw_verify_area+0x76/0x97
     [<f8f5553a>] ? device_write+0x0/0x432 [dlm]
     [<c04832bb>] ? vfs_write+0x8a/0x12e
     [<c04817e7>] ? do_sys_open+0xab/0xb5
     [<c04833f8>] ? sys_write+0x3b/0x60
     [<c0405bf2>] ? syscall_call+0x7/0xb
     [<c0620000>] ? acpi_pci_root_add+0x22f/0x2a0
     =======================
    Code: <snipped>
    EIP: [<f8f555df>] device_write+0xa5/0x432 [dlm] SS:ESP 0068:c8a45f44
    ---[ end trace 74c3a9c3bd1a789d ]---
    [yamato at localhost dlm-crash]$ 



Here is a patch.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>

diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index ebbcf38..1aa76b3 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -538,7 +538,7 @@ static ssize_t device_write(struct file *file, const char __user *buf,
 
 	/* do we really need this? can a write happen after a close? */
 	if ((kbuf->cmd == DLM_USER_LOCK || kbuf->cmd == DLM_USER_UNLOCK) &&
-	    test_bit(DLM_PROC_FLAGS_CLOSING, &proc->flags))
+	    (proc && test_bit(DLM_PROC_FLAGS_CLOSING, &proc->flags)))
 		return -EINVAL;
 
 	sigfillset(&allsigs);