All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: christophe.ricard@gmail.com
Cc: linux-wireless@vger.kernel.org
Subject: re: NFC: st21nfca: Add HCI transaction event support
Date: Tue, 10 Feb 2015 12:00:51 +0300	[thread overview]
Message-ID: <20150210090051.GA31867@mwanda> (raw)

Hello Christophe Ricard,

The patch 26fc6c7f02cb: "NFC: st21nfca: Add HCI transaction event
support" from Feb 1, 2015, leads to the following static checker
warning:

	drivers/nfc/st21nfca/st21nfca_se.c:321 st21nfca_connectivity_event_received()
	error: 'skb->data[1]' from user is not capped properly

drivers/nfc/st21nfca/st21nfca_se.c
   300  int st21nfca_connectivity_event_received(struct nfc_hci_dev *hdev, u8 host,
   301                                  u8 event, struct sk_buff *skb)
   302  {
   303          int r = 0;
   304          struct device *dev = &hdev->ndev->dev;
   305          struct nfc_evt_transaction *transaction;
   306  
   307          pr_debug("connectivity gate event: %x\n", event);
   308  
   309          switch (event) {
   310          case ST21NFCA_EVT_CONNECTIVITY:
   311                  break;
   312          case ST21NFCA_EVT_TRANSACTION:
   313                  if (skb->len < NFC_MIN_AID_LENGTH + 2 &&
   314                      skb->data[0] != NFC_EVT_TRANSACTION_AID_TAG)
   315                          return -EPROTO;

Here we don't trust skb->data[0].

   316  
   317                  transaction = (struct nfc_evt_transaction *)devm_kzalloc(dev,
   318                                                     skb->len - 2, GFP_KERNEL);
   319  
   320                  transaction->aid_len = skb->data[1];
   321                  memcpy(transaction->aid, &skb->data[2], skb->data[1]);
                                                                ^^^^^^^^^^^^
But here we trust skb->data[1].

NFC code is hard to analyze because sometimes skb->data[] comes from the
firmware and holds trusted values.  But sometimes it comes from the
network and can overflow.  Smatch marks it all as untrusted so it causes
a lot of false postives.

Some of them have comments like:

	net/nfc/hci/core.c:218 nfc_hci_cmd_received()
	error: buffer overflow 'hdev->pipes' 127 <= 255

But this one doesn't have a comment so it's hard for me as an outsider
to say if this is a bug or not.

   322  

regards,
dan carpenter

             reply	other threads:[~2015-02-10  8:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-10  9:00 Dan Carpenter [this message]
2015-06-05 10:59 ` NFC: st21nfca: Add HCI transaction event support Dan Carpenter
2015-06-05 11:05   ` christophe.ricard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150210090051.GA31867@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=christophe.ricard@gmail.com \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.