[Bug 209407] cdc_ncm_fill_tx_frame calls memset with huge length when called from cdc_mbim_tx_fixup

[bugzilla-daemon@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=209407

--- Comment #8 from Jouni Seppänen (jks@iki.fi) ---
I can't claim to understand what all the code in these files does, but I
noticed a couple of things:

The functions cdc_ncm_ndp16 and cdc_ncm_ndp32 include this check:

        /* verify that there is room for the NDP and the datagram (reserve) */
        if ((ctx->tx_curr_size - skb->len - reserve) < ctx->max_ndp_size)
                return NULL;

I think the types on the left-hand side of the comparison are as follows:

struct cdc_ncm_ctx {
        ...
        u32 tx_curr_size;
        ...
}

struct sk_buff {
        ...
        unsigned int            len,
                                data_len;
        ...
}

static struct usb_cdc_ncm_ndp16 *cdc_ncm_ndp16(..., size_t reserve)

Each type is unsigned, so if skb->len + reserve ever happens to be greater than
ctx->tx_curr_size, the subtraction would result in a large number and the
comparison would almost certainly yield false.

Another thing: there seems to be a set of constants referring to 16/32 bit
quantities, and quantities of the same size seem to go together. For example,
when cdc_ncm_tx_fixup calls cdc_ncm_fill_tx_frame, it chooses the sign argument
like so:

        if (ctx->is_ndp16)
                skb_out = cdc_ncm_fill_tx_frame(dev, skb,
cpu_to_le32(USB_CDC_NCM_NDP16_NOCRC_SIGN));
        else
                skb_out = cdc_ncm_fill_tx_frame(dev, skb,
cpu_to_le32(USB_CDC_NCM_NDP32_NOCRC_SIGN));

But when cdc_mbim_tx_fixup calls cdc_ncm_fill_tx_frame, the sign argument is
either cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN) or
cpu_to_le32(USB_CDC_MBIM_NDP16_DSS_SIGN). The constants
USB_CDC_MBIM_NDP32_IPS_SIGN and USB_CDC_MBIM_NDP32_DSS_SIGN are defined in
include/uapi/linux/usb/cdc.h but neither seems to be used anywhere in the
kernel.

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.