2012-09-26 14:06:08

by Dan Carpenter

Subject: re: NFC: Set local general bytes in nci_start_poll

Hello Ilan Elias,

The patch 7e0352306f68: "NFC: Set local general bytes in
nci_start_poll" from Aug 15, 2012, leads to the following warning:
net/nfc/nci/core.c:427 nci_set_local_general_bytes()
error: buffer overflow 'local_gb' 48 <= 250

416 __u8 local_gb[NFC_MAX_GT_LEN];
^^^^^^^^^^^^^^
48 elements.

417 int i, rc = 0;
418
419 param.val = nfc_get_local_general_bytes(nfc_dev, &param.len);
420 if ((param.val == NULL) || (param.len == 0))
421 return rc;
422
423 if (param.len > NCI_MAX_PARAM_LEN)
^^^^^^^^^^^^^^^^^
Capped at 250. Probably NFC_MAX_GT_LEN was intended?

424 return -EINVAL;
425
426 for (i = 0; i < param.len; i++)
427 local_gb[param.len-1-i] = param.val[i];
^^^^^^^^^^^^^
Writing to the 250th element.

This is just a sanity check and nfc_get_local_general_bytes() will only
return NFC_MAX_GT_LEN max because of the check in nfc_llcp_build_gb().

regards,
dan carpenter

2012-10-09 12:26:10

by Elias, Ilan

Subject: RE: NFC: Set local general bytes in nci_start_poll

Hi Dan,

Sorry for the late response, I was on vacation.

> Hello Ilan Elias,
>
> The patch 7e0352306f68: "NFC: Set local general bytes in
> nci_start_poll" from Aug 15, 2012, leads to the following warning:
> net/nfc/nci/core.c:427 nci_set_local_general_bytes()
> error: buffer overflow 'local_gb' 48 <= 250
>
> 416 __u8 local_gb[NFC_MAX_GT_LEN];
> ^^^^^^^^^^^^^^
> 48 elements.
>
> 417 int i, rc = 0;
> 418
> 419 param.val =
> nfc_get_local_general_bytes(nfc_dev, &param.len);
> 420 if ((param.val == NULL) || (param.len == 0))
> 421 return rc;
> 422
> 423 if (param.len > NCI_MAX_PARAM_LEN)
> ^^^^^^^^^^^^^^^^^
> Capped at 250. Probably NFC_MAX_GT_LEN was intended?
>
> 424 return -EINVAL;
> 425
> 426 for (i = 0; i < param.len; i++)
> 427 local_gb[param.len-1-i] = param.val[i];
> ^^^^^^^^^^^^^
> Writing to the 250th element.
>
> This is just a sanity check and nfc_get_local_general_bytes()
> will only
> return NFC_MAX_GT_LEN max because of the check in nfc_llcp_build_gb().

You're right.
I see now that Szymon Janc already sent a patch for this a few days ago.

Thanks & BR,
Ilan