[patch] cxt1e1: Correct Arbitrary memory write in c4_ioctl()

Vulnerability Description

The function c4_ioctl() (listed below) writes data from user in ifr->ifr_data to the kernel struct data arg, without performing any bounds checking. This allows using a crafted iocmd to write outside of the struct data arg, where iolen = IOC_SIZE(iocmd) can specify a maximum write size up to 2^14 bytes.

Triggering the write requires CAP_SYS_ADMIN capability but even in the case of having admin rights it should be disallowed.

Update

Starting with gcc-4.0, the gcc compiler allows to retrieve the size of an object GCC Object Size. The kernel uses the builtin_object_size() to implement checks on the copy size to prevent this class of vulnerabilities at copy_from_user_overflow() at arch/x86/include/asm/uaccess.h:690.

static inline unsigned long __must_check
688 copy_from_user(void *to, const void __user *from, unsigned long n)
689 {
690    int sz = __compiletime_object_size(to);
691
692    might_fault();
693
694    /*
695     * While we would like to have the compiler do the checking for us
696     * even in the non-constant size case, any false positives there are
697     * a problem (especially when DEBUG_STRICT_USER_COPY_CHECKS, but even
698     * without - the [hopefully] dangerous looking nature of the warning
699     * would make people go look at the respecitive call sites over and
700     * over again just to find that there's no problem).
701     *
702     * And there are cases where it's just not realistic for the compiler
703     * to prove the count to be in range. For example when multiple call
704     * sites of a helper function - perhaps in different source files -
705     * all doing proper range checking, yet the helper function not doing
706     * so again.
707     *
708     * Therefore limit the compile time checking to the constant size
709     * case, and do only runtime checking for non-constant sizes.
710     */
711
712    if (likely(sz < 0 || sz >= n))
713            n = _copy_from_user(to, from, n);
714    else if(__builtin_constant_p(n))
715            copy_from_user_overflow();
716    else
717            __copy_from_user_overflow(sz, n);
718
719    return n;

Fixing the Vulnerability

The proposed fix is to do bounds-checking of the iolen write size before performing the copy_from_user() to avoid the out-of-bounds data write. The patch fixing the vulnerability has been submitted to the Linux kernel:

Listing: c4_ioctl() function from drivers/staging/cxt1e1/linux.c commit 0414855f.

static      status_t
c4_ioctl (struct net_device *ndev, struct ifreq *ifr, int cmd)
{
    ci_t       *ci;
    void       *data;
    int         iocmd, iolen;
    status_t    ret;
    static struct data arg;

    if (!capable (CAP_SYS_ADMIN))
        return -EPERM;
    ...
    if (copy_from_user (&iocmd, ifr->ifr_data, sizeof (iocmd)))
        return -EFAULT;
    iolen = _IOC_SIZE (iocmd);
    data = ifr->ifr_data + sizeof (iocmd);
    if (copy_from_user (&arg, data, iolen))  // XXX iolen from user without bounds checks
        return -efault;