Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling SHDLC I-Frame commands

From: Suren Baghdasaryan
Date: Wed Aug 15 2018 - 12:40:19 EST


On Wed, Aug 15, 2018 at 1:29 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> On Tue, Aug 14, 2018 at 03:38:14PM -0700, Suren Baghdasaryan wrote:
>> The separate fix for the size of pipes[] array is posted here:
>> https://lkml.org/lkml/2018/8/14/1034
>> Thanks!
>>
>
> That's great! Let's add some bounds checking to nfc_hci_msg_rx_work()
> and nfc_hci_recv_from_llc() as well and then we can close the chapter on
> these bugs.

Dan, I don't think we need additional checks there. Here are the
relevant parts of the code in nfc_hci_recv_from_llc():

static void nfc_hci_recv_from_llc(struct nfc_hci_dev *hdev, struct sk_buff *skb)
{
...
packet = (struct hcp_packet *)skb->data;
...

/* it's the last fragment. Does it need re-aggregation? */
if (skb_queue_len(&hdev->rx_hcp_frags)) {
pipe = packet->header & NFC_HCI_FRAGMENT;
...
hcp_skb = nfc_alloc_recv_skb(NFC_HCI_HCP_PACKET_HEADER_LEN +
msg_len, GFP_KERNEL);
...
*skb_put(hcp_skb, NFC_HCI_HCP_PACKET_HEADER_LEN) = pipe;
...
} else {
packet->header &= NFC_HCI_FRAGMENT;
hcp_skb = skb;
}

AFAIU in both cases the pipe field in hcp_skb can't exceed 127 after
we applied NFC_HCI_FRAGMENT(0x7f) mask.

>
> regards,
> dan carpenter
>