Patchwork [10/22] bt: replace fragile snprintf use and unwarranted strncpy

login
register
mail settings
Submitter Jim Meyering
Date May 9, 2012, 9:23 a.m.
Message ID <1336555446-20180-11-git-send-email-jim@meyering.net>
Download mbox | patch
Permalink /patch/157983/
State New
Headers show

Comments

Jim Meyering - May 9, 2012, 9:23 a.m.
From: Jim Meyering <meyering@redhat.com>

In bt_hci_name_req a failed snprintf could return len larger than
sizeof(params.name), which means the following memset call would
have a "length" value of (size_t)-1, -2, etc...  Sounds scary.
But currently, one can deduce that there is no problem:
strlen(slave->lmp_name) is guaranteed to be smaller than
CHANGE_LOCAL_NAME_CP_SIZE, which is the same as sizeof(params.name),
so this cannot happen.  Regardless, there is no justification for
using snprintf+memset.  Use pstrcpy instead.

Also, in bt_hci_event_complete_read_local_name, use pstrcpy in place
of unwarranted strncpy.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 hw/bt-hci.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Patch

diff --git a/hw/bt-hci.c b/hw/bt-hci.c
index a3a7fb4..47f9a4e 100644
--- a/hw/bt-hci.c
+++ b/hw/bt-hci.c
@@ -943,7 +943,6 @@  static int bt_hci_name_req(struct bt_hci_s *hci, bdaddr_t *bdaddr)
 {
     struct bt_device_s *slave;
     evt_remote_name_req_complete params;
-    int len;

     for (slave = hci->device.net->slave; slave; slave = slave->next)
         if (slave->page_scan && !bacmp(&slave->bd_addr, bdaddr))
@@ -955,9 +954,7 @@  static int bt_hci_name_req(struct bt_hci_s *hci, bdaddr_t *bdaddr)

     params.status       = HCI_SUCCESS;
     bacpy(&params.bdaddr, &slave->bd_addr);
-    len = snprintf(params.name, sizeof(params.name),
-                    "%s", slave->lmp_name ?: "");
-    memset(params.name + len, 0, sizeof(params.name) - len);
+    pstrcpy(params.name, sizeof(params.name), slave->lmp_name ?: "");
     bt_hci_event(hci, EVT_REMOTE_NAME_REQ_COMPLETE,
                     &params, EVT_REMOTE_NAME_REQ_COMPLETE_SIZE);

@@ -1388,7 +1385,7 @@  static inline void bt_hci_event_complete_read_local_name(struct bt_hci_s *hci)
     params.status = HCI_SUCCESS;
     memset(params.name, 0, sizeof(params.name));
     if (hci->device.lmp_name)
-        strncpy(params.name, hci->device.lmp_name, sizeof(params.name));
+        pstrcpy(params.name, sizeof(params.name), hci->device.lmp_name);

     bt_hci_event_complete(hci, &params, READ_LOCAL_NAME_RP_SIZE);
 }