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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJuCfpH9n2vVjBF7RoMNng3T=URY1eusKfbVGyq44LWcjwNTPw@mail.gmail.com>
Date:   Wed, 15 Aug 2018 09:40:13 -0700
From:   Suren Baghdasaryan <surenb@...gle.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Kees Cook <keescook@...omium.org>,
        Security Officers <security@...nel.org>,
        Kevin Deus <kdeus@...gle.com>,
        Samuel Ortiz <sameo@...ux.intel.com>,
        "David S. Miller" <davem@...emloft.net>,
        Allen Pais <allen.pais@...cle.com>,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] NFC: Fix possible memory corruption when handling
 SHDLC I-Frame commands

On Wed, Aug 15, 2018 at 1:29 AM, Dan Carpenter <dan.carpenter@...cle.com> 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
>