NFC: Fix possible memory corruption when handling SHDLC I-Frame commands

PATCH In review Suren Baghdasaryan 4 participants 15 messages Aug 13, 2018 – Aug 17, 2018

Subsystems (3)

Networking Network drivers NFC

lore

Share & tools

lore

Suren Baghdasaryan

author

2018-08-13 22:39 UTC #0 lore raw

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

When handling SHDLC I-Frame commands "pipe" field used for indexing into an array should be checked before usage. If left unchecked it might access memory outside of the array of size NFC_HCI_MAX_PIPES(127).

Malformed NFC HCI frames could be injected by a malicious NFC device communicating with the device being attacked (remote attack vector), or even by an attacker with physical access to the I2C bus such that they could influence the data transfers on that bus (local attack vector). skb->data is controlled by the attacker and has only been sanitized in the most trivial ways (CRC check), therefore we can consider the create_info struct and all of its members to tainted. 'create_info->pipe' with max value of 255 (uint8) is used to take an offset of the hdev->pipes array of 127 elements which can lead to OOB write.

Signed-off-by Suren Baghdasaryan <[email protected]>

---
 net/nfc/hci/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

net/nfc/hci/core.c +10 -0

index ac8030c4bcf8..19cb2e473ea6 100644
@@ -209,6 +209,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
 		}
 		create_info = (struct hci_create_pipe_resp *)skb->data;
 
+		if (create_info->pipe >= NFC_HCI_MAX_PIPES) {
+			status = NFC_HCI_ANY_E_NOK;
+			goto exit;
+		}
+
 		/* Save the new created pipe and bind with local gate,
 		 * the description for skb->data[3] is destination gate id
 		 * but since we received this cmd from host controller, we

@@ -232,6 +237,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
 		}
 		delete_info = (struct hci_delete_pipe_noti *)skb->data;
 
+		if (delete_info->pipe >= NFC_HCI_MAX_PIPES) {
+			status = NFC_HCI_ANY_E_NOK;
+			goto exit;
+		}
+
 		hdev->pipes[delete_info->pipe].gate = NFC_HCI_INVALID_GATE;
 		hdev->pipes[delete_info->pipe].dest_host = NFC_HCI_INVALID_HOST;
 		break;

Signature · 1 line

2.18.0.597.ga71716f1ad-goog

Kees Cook

Acked-by on v1 1/1

2018-08-14 00:28 UTC #1 lore raw

Quoted 14 lines · Suren Baghdasaryan

When handling SHDLC I-Frame commands "pipe" field used for indexing into an array should be checked before usage. If left unchecked it might access memory outside of the array of size NFC_HCI_MAX_PIPES(127).
Malformed NFC HCI frames could be injected by a malicious NFC device communicating with the device being attacked (remote attack vector), or even by an attacker with physical access to the I2C bus such that they could influence the data transfers on that bus (local attack vector). skb->data is controlled by the attacker and has only been sanitized in the most trivial ways (CRC check), therefore we can consider the create_info struct and all of its members to tainted. 'create_info->pipe' with max value of 255 (uint8) is used to take an offset of the hdev->pipes array of 127 elements which can lead to OOB write.

Quoted 2 lines

Suggested-by Kevin Deus <[email protected]>
Signed-off-by Suren Baghdasaryan <[email protected]>

Nice find!

Acked-by Kees Cook <[email protected]>

Quoted 35 lines

---
 net/nfc/hci/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

net/nfc/hci/core.c +10 -1

index ac8030c4bcf8..19cb2e473ea6 100644
@@ -209,6 +209,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
                }
                create_info = (struct hci_create_pipe_resp *)skb->data;
 
+               if (create_info->pipe >= NFC_HCI_MAX_PIPES) {
+                       status = NFC_HCI_ANY_E_NOK;
+                       goto exit;
+               }
+
                /* Save the new created pipe and bind with local gate,
                 * the description for skb->data[3] is destination gate id
                 * but since we received this cmd from host controller, we

@@ -232,6 +237,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
                }
                delete_info = (struct hci_delete_pipe_noti *)skb->data;
 
+               if (delete_info->pipe >= NFC_HCI_MAX_PIPES) {
+                       status = NFC_HCI_ANY_E_NOK;
+                       goto exit;
+               }
+
                hdev->pipes[delete_info->pipe].gate = NFC_HCI_INVALID_GATE;
                hdev->pipes[delete_info->pipe].dest_host = NFC_HCI_INVALID_HOST;
                break;
--

2.18.0.597.ga71716f1ad-goog

Signature · 2 lines

Kees Cook
Pixel Security

Greg KH

Reviewed-by Greg Kroah-Hartman on v1 1/1

2018-08-14 09:21 UTC #2 lore raw

In reply to Suren Baghdasaryan

Quoted 14 lines · Suren Baghdasaryan

When handling SHDLC I-Frame commands "pipe" field used for indexing into an array should be checked before usage. If left unchecked it might access memory outside of the array of size NFC_HCI_MAX_PIPES(127).
Malformed NFC HCI frames could be injected by a malicious NFC device communicating with the device being attacked (remote attack vector), or even by an attacker with physical access to the I2C bus such that they could influence the data transfers on that bus (local attack vector). skb->data is controlled by the attacker and has only been sanitized in the most trivial ways (CRC check), therefore we can consider the create_info struct and all of its members to tainted. 'create_info->pipe' with max value of 255 (uint8) is used to take an offset of the hdev->pipes array of 127 elements which can lead to OOB write.

Quoted 2 lines

Suggested-by Kevin Deus <[email protected]>
Signed-off-by Suren Baghdasaryan <[email protected]>

Reviewed-by Greg Kroah-Hartman <[email protected]>

Dan Carpenter

2018-08-14 09:54 UTC #3 lore raw

In reply to Suren Baghdasaryan

Thanks. This is great. I'm so glad these are finally getting fixed.

Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.

Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.

regards,
dan carpenter

Suren Baghdasaryan

author

2018-08-14 16:57 UTC #4 lore raw

Quoted 9 lines · Dan Carpenter

Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.

Good point. From hci.h:

/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127

And then:

struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}

I think the correct fix would be to change it to:

  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];

What do you think?

Quoted 4 lines

regards,
dan carpenter

Suren Baghdasaryan

author

2018-08-14 20:26 UTC #5 lore raw

Quoted 27 lines · Suren Baghdasaryan

Quoted 9 lines · Dan Carpenter
Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.
Good point. From hci.h:
/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127
And then:
struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}

Quoted 6 lines

I think the correct fix would be to change it to:
  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
What do you think?

Just to be clear, this would fix the problem Dan described in his reply and it should be implemented in a separate patch. The original fix is still valid.

Quoted 4 lines

Quoted 4 lines
regards,
dan carpenter

Kees Cook

2018-08-14 20:33 UTC #6 lore raw

Quoted 30 lines · Suren Baghdasaryan

Quoted 29 lines · Suren Baghdasaryan
Quoted 9 lines · Dan Carpenter
Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.
Good point. From hci.h:
/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127
And then:
struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}
I think the correct fix would be to change it to:

Quoted 8 lines

Quoted 4 lines
  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
What do you think?
Just to be clear, this would fix the problem Dan described in his reply and it should be implemented in a separate patch. The original fix is still valid.

I think you could merge the fixes into a single patch.

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Suren Baghdasaryan

author

2018-08-14 20:55 UTC #7 lore raw

Quoted 33 lines · Kees Cook

Quoted 32 lines · Suren Baghdasaryan
Quoted 31 lines · Suren Baghdasaryan
Quoted 9 lines · Dan Carpenter
Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.
Good point. From hci.h:
/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127
And then:
struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}
I think the correct fix would be to change it to:
  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
What do you think?
Just to be clear, this would fix the problem Dan described in his reply and it should be implemented in a separate patch. The original fix is still valid.
I think you could merge the fixes into a single patch.

Couple reasons I would prefer to keep them separate:
- I feel that descriptions for these two issues should be different and it's easier if we don't mix them up
- This one is already merged into Android kernels, so would be easier to introduce the second fix separately
- I would like to give credit to people who noticed the problem (in this case those are different people)

However if more people think we should fix both issues in the same patch I'll happily do that.
Thanks!

Quoted 6 lines

-Kees
--
Kees Cook
Pixel Security

Kees Cook

2018-08-14 21:49 UTC #8 lore raw

Quoted 51 lines · Suren Baghdasaryan

Quoted 33 lines · Kees Cook
Quoted 32 lines · Suren Baghdasaryan
Quoted 31 lines · Suren Baghdasaryan
Quoted 9 lines · Dan Carpenter
Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.
Good point. From hci.h:
/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127
And then:
struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}
I think the correct fix would be to change it to:
  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
What do you think?
Just to be clear, this would fix the problem Dan described in his reply and it should be implemented in a separate patch. The original fix is still valid.
I think you could merge the fixes into a single patch.
Couple reasons I would prefer to keep them separate:
- I feel that descriptions for these two issues should be different and it's easier if we don't mix them up
- This one is already merged into Android kernels, so would be easier to introduce the second fix separately
- I would like to give credit to people who noticed the problem (in this case those are different people)

Quoted 3 lines

However if more people think we should fix both issues in the same patch I'll happily do that.
Thanks!

If it's already landed separately somewhere else, then yeah, 2 patches sounds good. No objection either way from me!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Suren Baghdasaryan

author

2018-08-14 22:19 UTC #9 lore raw

Quoted 31 lines · Kees Cook

Quoted 30 lines · Suren Baghdasaryan
Quoted 29 lines · Kees Cook
Quoted 28 lines · Suren Baghdasaryan
Quoted 27 lines · Suren Baghdasaryan
Quoted 9 lines · Dan Carpenter
Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.
Good point. From hci.h:
/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127
And then:
struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
I think the correct fix would be to change it to:
  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
What do you think?

Actually, after looking more closely, NFC_HCI_MAX_PIPES is always used as the number of supported pipes and not as the max pipe ID, so the right fix would be:

-#define NFC_HCI_MAX_PIPES 127
+#define NFC_HCI_MAX_PIPES 128

I would prefer to rename it into NFC_HCI_PIPE_COUNT but don't want to introduce unnecessary churn for one-line change, so will keep the name. Will post a separate fix for this shortly.

Quoted 27 lines

Quoted 15 lines
Quoted 6 lines
Quoted 4 lines
Just to be clear, this would fix the problem Dan described in his reply and it should be implemented in a separate patch. The original fix is still valid.
I think you could merge the fixes into a single patch.
Couple reasons I would prefer to keep them separate:
- I feel that descriptions for these two issues should be different and it's easier if we don't mix them up
- This one is already merged into Android kernels, so would be easier to introduce the second fix separately
- I would like to give credit to people who noticed the problem (in this case those are different people)
Quoted 3 lines
However if more people think we should fix both issues in the same patch I'll happily do that.
Thanks!
If it's already landed separately somewhere else, then yeah, 2 patches sounds good. No objection either way from me!
-Kees
--
Kees Cook
Pixel Security

Suren Baghdasaryan

author

2018-08-14 22:38 UTC #10 lore raw

The separate fix for the size of pipes[] array is posted here:
https://lkml.org/lkml/2018/8/14/1034
Thanks!

Quoted 77 lines · Suren Baghdasaryan

Quoted 31 lines · Kees Cook
Quoted 30 lines · Suren Baghdasaryan
Quoted 29 lines · Kees Cook
Quoted 28 lines · Suren Baghdasaryan
Quoted 27 lines · Suren Baghdasaryan
Quoted 9 lines · Dan Carpenter
Thanks. This is great. I'm so glad these are finally getting fixed.
Do we need to fix nfc_hci_msg_rx_work() and nfc_hci_recv_from_llc() as well? In nfc_hci_recv_from_llc() we allow pipe to be NFC_HCI_FRAGMENT (0x7f) so that's one element beyond the end of the array and the NFC_HCI_HCP_RESPONSE isn't checked.
Also nci_hci_msg_rx_work() and nci_hci_data_received_cb() use NCI_HCP_MSG_GET_PIPE() so those could be off by one.
Good point. From hci.h:
/*
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
#define NFC_HCI_MAX_PIPES 127
And then:
struct nfc_hci_dev {
...
struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES];
...
}
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
I think the correct fix would be to change it to:
  struct nfc_hci_pipe pipes[NFC_HCI_MAX_PIPES + 1];
What do you think?
Actually, after looking more closely, NFC_HCI_MAX_PIPES is always used as the number of supported pipes and not as the max pipe ID, so the right fix would be:
(quoted diff) +1 -1
-#define NFC_HCI_MAX_PIPES 127
+#define NFC_HCI_MAX_PIPES 128
I would prefer to rename it into NFC_HCI_PIPE_COUNT but don't want to introduce unnecessary churn for one-line change, so will keep the name. Will post a separate fix for this shortly.
Quoted 27 lines
Quoted 15 lines
Quoted 6 lines
Quoted 4 lines
Just to be clear, this would fix the problem Dan described in his reply and it should be implemented in a separate patch. The original fix is still valid.
I think you could merge the fixes into a single patch.
Couple reasons I would prefer to keep them separate:
- I feel that descriptions for these two issues should be different and it's easier if we don't mix them up
- This one is already merged into Android kernels, so would be easier to introduce the second fix separately
- I would like to give credit to people who noticed the problem (in this case those are different people)
Quoted 3 lines
However if more people think we should fix both issues in the same patch I'll happily do that.
Thanks!
If it's already landed separately somewhere else, then yeah, 2 patches sounds good. No objection either way from me!
-Kees
--
Kees Cook
Pixel Security

Dan Carpenter

2018-08-15 08:29 UTC #11 lore raw

Quoted 4 lines · Suren Baghdasaryan

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.

regards,
dan carpenter

Suren Baghdasaryan

author

2018-08-15 16:40 UTC #12 lore raw

Quoted 9 lines · Dan Carpenter

Quoted 4 lines · Suren Baghdasaryan
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.

Quoted 4 lines

regards,
dan carpenter

Dan Carpenter

2018-08-17 12:07 UTC #13 lore raw

Quoted 7 lines · Suren Baghdasaryan

Quoted 6 lines · Dan Carpenter
Quoted 4 lines · Suren Baghdasaryan
The separate fix for the size of pipes[] array is posted here:
https://lkml.org/lkml/2018/8/14/1034
Thanks!

Quoted 7 lines

Quoted 3 lines
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():

Sorry, I meant after that at the end of the function:

net/nfc/hci/core.c
902 /* if this is a response, dispatch immediately to

   903           * unblock waiting cmd context. Otherwise, enqueue to dispatch
   904           * in separate context where handler can also execute command.
   905           */
   906          packet = (struct hcp_packet *)hcp_skb->data;
   907          type = HCP_MSG_GET_TYPE(packet->message.header);
   908          if (type == NFC_HCI_HCP_RESPONSE) {
   909                  pipe = packet->header;
                        ^^^^^^^^^^^^^^^^^^^^^

Pipe can go up to 255.

910 instruction = HCP_MSG_GET_CMD(packet->message.header);
911 skb_pull(hcp_skb, NFC_HCI_HCP_PACKET_HEADER_LEN +
912 NFC_HCI_HCP_MESSAGE_HEADER_LEN);

   913                  nfc_hci_hcp_message_rx(hdev, pipe, type, instruction, hcp_skb);
                                                     ^^^^

Then inside the nfc_hci_hcp_message_rx() function we call nfc_hci_cmd_received() and nfc_hci_event_received() which use it as an array index.

   914          } else {
   915                  skb_queue_tail(&hdev->msg_rx_queue, hcp_skb);
   916                  schedule_work(&hdev->msg_rx_work);
   917          }
   918  }

It's the same thing when nfc_hci_hcp_message_rx() is called from nfc_hci_msg_rx_work():

   138  static void nfc_hci_msg_rx_work(struct work_struct *work)
   139  {
   140          struct nfc_hci_dev *hdev = container_of(work, struct nfc_hci_dev,
   141                                                  msg_rx_work);
   142          struct sk_buff *skb;
   143          struct hcp_message *message;
   144          u8 pipe;
   145          u8 type;
   146          u8 instruction;
   147  
   148          while ((skb = skb_dequeue(&hdev->msg_rx_queue)) != NULL) {
   149                  pipe = skb->data[0];
                        ^^^^^^^^^^^^^^^^^^^
   150                  skb_pull(skb, NFC_HCI_HCP_PACKET_HEADER_LEN);
   151                  message = (struct hcp_message *)skb->data;
   152                  type = HCP_MSG_GET_TYPE(message->header);
   153                  instruction = HCP_MSG_GET_CMD(message->header);
   154                  skb_pull(skb, NFC_HCI_HCP_MESSAGE_HEADER_LEN);
   155  
   156                  nfc_hci_hcp_message_rx(hdev, pipe, type, instruction, skb);
                                                     ^^^^
   157          }
   158  }

regards,
dan carpenter

Dan Carpenter

2018-08-17 14:47 UTC #14 lore raw

Never mind. This code is a bit subtle for static analysis so those warnings are false positives. Thanks for taking a look at this, Suren.

regards,
dan carpenter