CVE-2011-4077: Linux kernel XFS readlink() Memory Corruption

This interesting vulnerability was recently reported by Carlos Maiolino on xfs mailing list. This vulnerability becomes critical on kernels that do not have ‘CONFIG_XFS_DEBUG’ option enabled and we will see now why.
The bug is part of fs/xfs/xfs_vnodeops.c file in the C routine you see below.

int
xfs_readlink(
        xfs_inode_t     *ip,
        char            *link)
{
        xfs_mount_t     *mp = ip->i_mount;
        int             pathlen;
        int             error = 0;

        trace_xfs_readlink(ip);

        if (XFS_FORCED_SHUTDOWN(mp))
                return XFS_ERROR(EIO);

        xfs_ilock(ip, XFS_ILOCK_SHARED);

        ASSERT(S_ISLNK(ip->i_d.di_mode));
        ASSERT(ip->i_d.di_size <= MAXPATHLEN);

        pathlen = ip->i_d.di_size;
        if (!pathlen)
                goto out;

        if (ip->i_df.if_flags & XFS_IFINLINE) {
                memcpy(link, ip->i_df.if_u1.if_data, pathlen);
                link[pathlen] = '\0';
        } else {
                error = xfs_readlink_bmap(ip, link);
        }

 out:
        xfs_iunlock(ip, XFS_ILOCK_SHARED);
        return error;
}

As you can see, there are two ASSERT() calls to ensure that the file is a symbolic link and that it does not exceed the ‘MAXPATHLEN’ length. However, if the kernel is not compiled with ‘CONFIG_XFS_DEBUG’ and a symbolic link longer than ‘MAXPATHLEN’ is used on an XFS image, then the two ASSERT() checks won’t be executed and the subsequent memcpy() call will result in heap memory corruption since ‘pathlen’ is initialized directly with the file’s size via ‘ip->i_d.di_size’ variable.

The fix to this vulnerability was to remove the two ASSERT() calls…

        xfs_ilock(ip, XFS_ILOCK_SHARED);
 
-       ASSERT(S_ISLNK(ip->i_d.di_mode));
-       ASSERT(ip->i_d.di_size <= MAXPATHLEN);
-
        pathlen = ip->i_d.di_size;

Since the first check is performed on the VFS layer and xfs_readlink_by_handle() prior to this routine and the second one becauce the following code was added to always check for maximum path length.

                goto out;
 
+       if (pathlen > MAXPATHLEN) {
+               xfs_alert(mp, "%s: inode (%llu) symlink length (%d) too long",
+                        __func__, (unsigned long long)ip->i_ino, pathlen);
+               ASSERT(0);
+               return XFS_ERROR(EFSCORRUPTED);
+       }
+
+
        if (ip->i_df.if_flags & XFS_IFINLINE) {