xorl %eax, %eax

Linux kernel CIFS Invalid Pointer Free

leave a comment »

This bug, found in 2.6.32-rc6’s ChangeLog file was reported by Hitoshi Mitake of WASEDA University and here is the buggy code from 2.6.31 release of the Linux kernel…

static struct TCP_Server_Info *
cifs_get_tcp_session(struct smb_vol *volume_info)
{
        struct TCP_Server_Info *tcp_ses = NULL;
        struct sockaddr_storage addr;
        struct sockaddr_in *sin_server = (struct sockaddr_in *) &addr;
        struct sockaddr_in6 *sin_server6 = (struct sockaddr_in6 *) &addr;
        int rc;

        memset(&addr, 0, sizeof(struct sockaddr_storage));
     ...
out_err:
        if (tcp_ses) {
                kfree(tcp_ses->hostname);
                if (tcp_ses->ssocket)
                        sock_release(tcp_ses->ssocket);
                kfree(tcp_ses);
        }
        return ERR_PTR(rc);
}

The above function resides in fs/cifs/connect.c and it is used to return a pointer to a ‘TCP_Server_Info’ structure containing the TCP session’s information. As you can read, ‘out_err’ handles the exceptional conditions which could appear in various situations including: failure of address translation, unspecified UNC path, if there is already a TCP session, if it fails to extract the hostname from the TCP session etc.
However, the ‘out_err’ label code will attempt to kfree() the ‘tcp_ses->hostname’ regardless of its state. As we can read from fs/cifs/cifsglob.h this member stores the following information:

struct TCP_Server_Info {
     ...
        char *hostname; /* hostname portion of UNC string */
     ...
};

So, in case of a TCP session with UNC not set, the following code will attempt to call kfree() on an invalid pointer. To fix this, the following patch was used:

 	if (tcp_ses) {
-		kfree(tcp_ses->hostname);
+		if (!IS_ERR(tcp_ses->hostname))
+			kfree(tcp_ses->hostname);
 		if (tcp_ses->ssocket)

The IS_ERR() macro is defined in include/linux/err.h and as you can read below is a simple range check with branch prediction code for optimization purposes.

#define MAX_ERRNO       4095
     ...
#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
     ...
static inline long IS_ERR(const void *ptr)
{
        return IS_ERR_VALUE((unsigned long)ptr);
}

As H. Mitake said, ‘tcp_ses->hostname’ member is normally initialized by extract_hostname() using kmalloc() like this:

/* extract the host portion of the UNC string */
static char *
extract_hostname(const char *unc)
{
        const char *src;
        char *dst, *delim;
        unsigned int len;

        /* skip double chars at beginning of string */
        /* BB: check validity of these bytes? */
        src = unc + 2;

        /* delimiter between hostname and sharename is always '\\' now */
        delim = strchr(src, '\\');
        if (!delim)
                return ERR_PTR(-EINVAL);

        len = delim - src;
        dst = kmalloc((len + 1), GFP_KERNEL);
        if (dst == NULL)
                return ERR_PTR(-ENOMEM);

        memcpy(dst, src, len);
        dst[len] = '';

        return dst;
}

But cifs_get_tcp_session() will call kfree() regardless of the allocation’s success or failure.

Written by xorl

November 7, 2009 at 01:22

Posted in bugs, linux

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