All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] bt: use size_t type for length parameters instead of signed int
@ 2018-10-17 20:58 P J P
  2018-10-18 10:59 ` Paolo Bonzini
  0 siblings, 1 reply; 3+ messages in thread
From: P J P @ 2018-10-17 20:58 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Arash TC, Paolo Bonzini, Philippe Mathieu Daude, Prasad J Pandit

From: Prasad J Pandit <pjp@fedoraproject.org>

The length parameter values are not negative, thus use an unsigned
type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
calls. If it was negative, it could lead to memory corruption issues.

Reported-by: Arash TC <tohidi.arash@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 bt-host.c              |  6 ++---
 bt-vhci.c              |  4 +--
 hw/bt/core.c           |  2 +-
 hw/bt/hci-csr.c        | 16 ++++++------
 hw/bt/hci.c            | 38 ++++++++++++++--------------
 hw/bt/hid.c            |  8 +++---
 hw/bt/l2cap.c          | 56 ++++++++++++++++++++++--------------------
 hw/bt/sdp.c            |  6 ++---
 hw/usb/dev-bluetooth.c |  6 ++---
 include/hw/bt.h        |  8 +++---
 include/sysemu/bt.h    | 10 ++++----
 11 files changed, 81 insertions(+), 79 deletions(-)

This change is similar to
  -> https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02402.html

diff --git a/bt-host.c b/bt-host.c
index 2f8f631c25..9a3e3e4d2a 100644
--- a/bt-host.c
+++ b/bt-host.c
@@ -63,17 +63,17 @@ static void bt_host_send(struct HCIInfo *hci,
     }
 }
 
-static void bt_host_cmd(struct HCIInfo *hci, const uint8_t *data, int len)
+static void bt_host_cmd(struct HCIInfo *hci, const uint8_t *data, size_t len)
 {
     bt_host_send(hci, HCI_COMMAND_PKT, data, len);
 }
 
-static void bt_host_acl(struct HCIInfo *hci, const uint8_t *data, int len)
+static void bt_host_acl(struct HCIInfo *hci, const uint8_t *data, size_t len)
 {
     bt_host_send(hci, HCI_ACLDATA_PKT, data, len);
 }
 
-static void bt_host_sco(struct HCIInfo *hci, const uint8_t *data, int len)
+static void bt_host_sco(struct HCIInfo *hci, const uint8_t *data, size_t len)
 {
     bt_host_send(hci, HCI_SCODATA_PKT, data, len);
 }
diff --git a/bt-vhci.c b/bt-vhci.c
index 9d277c32bf..534c4ed209 100644
--- a/bt-vhci.c
+++ b/bt-vhci.c
@@ -125,13 +125,13 @@ static void vhci_host_send(void *opaque,
 }
 
 static void vhci_out_hci_packet_event(void *opaque,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     vhci_host_send(opaque, HCI_EVENT_PKT, data, len);
 }
 
 static void vhci_out_hci_packet_acl(void *opaque,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     vhci_host_send(opaque, HCI_ACLDATA_PKT, data, len);
 }
diff --git a/hw/bt/core.c b/hw/bt/core.c
index 78370e64f5..62720d1663 100644
--- a/hw/bt/core.c
+++ b/hw/bt/core.c
@@ -45,7 +45,7 @@ static void bt_dummy_lmp_disconnect_master(struct bt_link_s *link)
 }
 
 static void bt_dummy_lmp_acl_resp(struct bt_link_s *link,
-                const uint8_t *data, int start, int len)
+                const uint8_t *data, int start, size_t len)
 {
     error_report("%s: stray ACL response PDU, fixme", __func__);
     exit(-1);
diff --git a/hw/bt/hci-csr.c b/hw/bt/hci-csr.c
index 0341ded50c..c90e5d3dc4 100644
--- a/hw/bt/hci-csr.c
+++ b/hw/bt/hci-csr.c
@@ -103,7 +103,7 @@ static inline void csrhci_fifo_wake(struct csrhci_s *s)
 }
 
 #define csrhci_out_packetz(s, len) memset(csrhci_out_packet(s, len), 0, len)
-static uint8_t *csrhci_out_packet(struct csrhci_s *s, int len)
+static uint8_t *csrhci_out_packet(struct csrhci_s *s, size_t len)
 {
     int off = s->out_start + s->out_len;
 
@@ -112,14 +112,14 @@ static uint8_t *csrhci_out_packet(struct csrhci_s *s, int len)
 
     if (off < FIFO_LEN) {
         if (off + len > FIFO_LEN && (s->out_size = off + len) > FIFO_LEN * 2) {
-            error_report("%s: can't alloc %i bytes", __func__, len);
+            error_report("%s: can't alloc %zu bytes", __func__, len);
             exit(-1);
         }
         return s->outfifo + off;
     }
 
     if (s->out_len > s->out_size) {
-        error_report("%s: can't alloc %i bytes", __func__, len);
+        error_report("%s: can't alloc %zu bytes", __func__, len);
         exit(-1);
     }
 
@@ -127,7 +127,7 @@ static uint8_t *csrhci_out_packet(struct csrhci_s *s, int len)
 }
 
 static inline uint8_t *csrhci_out_packet_csr(struct csrhci_s *s,
-                int type, int len)
+                int type, size_t len)
 {
     uint8_t *ret = csrhci_out_packetz(s, len + 2);
 
@@ -138,7 +138,7 @@ static inline uint8_t *csrhci_out_packet_csr(struct csrhci_s *s,
 }
 
 static inline uint8_t *csrhci_out_packet_event(struct csrhci_s *s,
-                int evt, int len)
+                int evt, size_t len)
 {
     uint8_t *ret = csrhci_out_packetz(s,
                     len + 1 + sizeof(struct hci_event_hdr));
@@ -151,7 +151,7 @@ static inline uint8_t *csrhci_out_packet_event(struct csrhci_s *s,
 }
 
 static void csrhci_in_packet_vendor(struct csrhci_s *s, int ocf,
-                uint8_t *data, int len)
+                uint8_t *data, size_t len)
 {
     int offset;
     uint8_t *rpkt;
@@ -363,7 +363,7 @@ static int csrhci_write(struct Chardev *chr,
 }
 
 static void csrhci_out_hci_packet_event(void *opaque,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     struct csrhci_s *s = (struct csrhci_s *) opaque;
     uint8_t *pkt = csrhci_out_packet(s, (len + 2) & ~1);	/* Align */
@@ -375,7 +375,7 @@ static void csrhci_out_hci_packet_event(void *opaque,
 }
 
 static void csrhci_out_hci_packet_acl(void *opaque,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     struct csrhci_s *s = (struct csrhci_s *) opaque;
     uint8_t *pkt = csrhci_out_packet(s, (len + 2) & ~1);	/* Align */
diff --git a/hw/bt/hci.c b/hw/bt/hci.c
index c6b2cc1d48..c59ccc55b9 100644
--- a/hw/bt/hci.c
+++ b/hw/bt/hci.c
@@ -32,7 +32,7 @@
 
 struct bt_hci_s {
     uint8_t *(*evt_packet)(void *opaque);
-    void (*evt_submit)(void *opaque, int len);
+    void (*evt_submit)(void *opaque, size_t len);
     void *opaque;
     uint8_t evt_buf[256];
 
@@ -62,7 +62,7 @@ struct bt_hci_s {
         struct bt_hci_master_link_s {
             struct bt_link_s *link;
             void (*lmp_acl_data)(struct bt_link_s *link,
-                            const uint8_t *data, int start, int len);
+                            const uint8_t *data, int start, size_t len);
             QEMUTimer *acl_mode_timer;
         } handle[HCI_HANDLES_MAX];
         uint32_t role_bmp;
@@ -434,7 +434,7 @@ static const uint8_t bt_event_reserved_mask[8] = {
 };
 
 
-static void null_hci_send(struct HCIInfo *hci, const uint8_t *data, int len)
+static void null_hci_send(struct HCIInfo *hci, const uint8_t *data, size_t len)
 {
 }
 
@@ -452,13 +452,13 @@ struct HCIInfo null_hci = {
 
 
 static inline uint8_t *bt_hci_event_start(struct bt_hci_s *hci,
-                int evt, int len)
+                int evt, size_t len)
 {
     uint8_t *packet, mask;
     int mask_byte;
 
     if (len > 255) {
-        error_report("%s: HCI event params too long (%ib)", __func__, len);
+        error_report("%s: HCI event params too long (%zub)", __func__, len);
         exit(-1);
     }
 
@@ -475,7 +475,7 @@ static inline uint8_t *bt_hci_event_start(struct bt_hci_s *hci,
 }
 
 static inline void bt_hci_event(struct bt_hci_s *hci, int evt,
-                void *params, int len)
+                void *params, size_t len)
 {
     uint8_t *packet = bt_hci_event_start(hci, evt, len);
 
@@ -500,7 +500,7 @@ static inline void bt_hci_event_status(struct bt_hci_s *hci, int status)
 }
 
 static inline void bt_hci_event_complete(struct bt_hci_s *hci,
-                void *ret, int len)
+                void *ret, size_t len)
 {
     uint8_t *packet = bt_hci_event_start(hci, EVT_CMD_COMPLETE,
                     len + EVT_CMD_COMPLETE_SIZE);
@@ -1477,7 +1477,7 @@ static inline void bt_hci_event_num_comp_pkts(struct bt_hci_s *hci,
 }
 
 static void bt_submit_hci(struct HCIInfo *info,
-                const uint8_t *data, int length)
+                const uint8_t *data, size_t length)
 {
     struct bt_hci_s *hci = hci_from_info(info);
     uint16_t cmd;
@@ -1971,7 +1971,7 @@ static void bt_submit_hci(struct HCIInfo *info,
         break;
 
     short_hci:
-        error_report("%s: HCI packet too short (%iB)", __func__, length);
+        error_report("%s: HCI packet too short (%zuB)", __func__, length);
         bt_hci_event_status(hci, HCI_INVALID_PARAMETERS);
         break;
     }
@@ -1982,7 +1982,7 @@ static void bt_submit_hci(struct HCIInfo *info,
  * know that a packet contained the last fragment of the SDU when the next
  * SDU starts.  */
 static inline void bt_hci_lmp_acl_data(struct bt_hci_s *hci, uint16_t handle,
-                const uint8_t *data, int start, int len)
+                const uint8_t *data, int start, size_t len)
 {
     struct hci_acl_hdr *pkt = (void *) hci->acl_buf;
 
@@ -1990,7 +1990,7 @@ static inline void bt_hci_lmp_acl_data(struct bt_hci_s *hci, uint16_t handle,
     /* TODO: avoid memcpy'ing */
 
     if (len + HCI_ACL_HDR_SIZE > sizeof(hci->acl_buf)) {
-        error_report("%s: can't take ACL packets %i bytes long",
+        error_report("%s: can't take ACL packets %zu bytes long",
                      __func__, len);
         return;
     }
@@ -2004,7 +2004,7 @@ static inline void bt_hci_lmp_acl_data(struct bt_hci_s *hci, uint16_t handle,
 }
 
 static void bt_hci_lmp_acl_data_slave(struct bt_link_s *btlink,
-                const uint8_t *data, int start, int len)
+                const uint8_t *data, int start, size_t len)
 {
     struct bt_hci_link_s *link = (struct bt_hci_link_s *) btlink;
 
@@ -2013,14 +2013,14 @@ static void bt_hci_lmp_acl_data_slave(struct bt_link_s *btlink,
 }
 
 static void bt_hci_lmp_acl_data_host(struct bt_link_s *link,
-                const uint8_t *data, int start, int len)
+                const uint8_t *data, int start, size_t len)
 {
     bt_hci_lmp_acl_data(hci_from_device(link->host),
                     link->handle, data, start, len);
 }
 
 static void bt_submit_acl(struct HCIInfo *info,
-                const uint8_t *data, int length)
+                const uint8_t *data, size_t length)
 {
     struct bt_hci_s *hci = hci_from_info(info);
     uint16_t handle;
@@ -2028,7 +2028,7 @@ static void bt_submit_acl(struct HCIInfo *info,
     struct bt_link_s *link;
 
     if (length < HCI_ACL_HDR_SIZE) {
-        error_report("%s: ACL packet too short (%iB)", __func__, length);
+        error_report("%s: ACL packet too short (%zuB)", __func__, length);
         return;
     }
 
@@ -2046,7 +2046,7 @@ static void bt_submit_acl(struct HCIInfo *info,
     handle &= ~HCI_HANDLE_OFFSET;
 
     if (datalen > length) {
-        error_report("%s: ACL packet too short (%iB < %iB)",
+        error_report("%s: ACL packet too short (%zuB < %iB)",
                      __func__, length, datalen);
         return;
     }
@@ -2088,7 +2088,7 @@ static void bt_submit_acl(struct HCIInfo *info,
 }
 
 static void bt_submit_sco(struct HCIInfo *info,
-                const uint8_t *data, int length)
+                const uint8_t *data, size_t length)
 {
     struct bt_hci_s *hci = hci_from_info(info);
     uint16_t handle;
@@ -2107,7 +2107,7 @@ static void bt_submit_sco(struct HCIInfo *info,
     }
 
     if (datalen > length) {
-        error_report("%s: SCO packet too short (%iB < %iB)",
+        error_report("%s: SCO packet too short (%zuB < %iB)",
                      __func__, length, datalen);
         return;
     }
@@ -2128,7 +2128,7 @@ static uint8_t *bt_hci_evt_packet(void *opaque)
     return s->evt_buf;
 }
 
-static void bt_hci_evt_submit(void *opaque, int len)
+static void bt_hci_evt_submit(void *opaque, size_t len)
 {
     /* TODO: notify upper layer */
     struct bt_hci_s *s = opaque;
diff --git a/hw/bt/hid.c b/hw/bt/hid.c
index 056291f9b5..e19f13c456 100644
--- a/hw/bt/hid.c
+++ b/hw/bt/hid.c
@@ -169,7 +169,7 @@ static void bt_hid_disconnect(struct bt_hid_device_s *s)
 }
 
 static void bt_hid_send_data(struct bt_l2cap_conn_params_s *ch, int type,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     uint8_t *pkt, hdr = (BT_DATA << 4) | type;
     int plen;
@@ -190,7 +190,7 @@ static void bt_hid_send_data(struct bt_l2cap_conn_params_s *ch, int type,
 }
 
 static void bt_hid_control_transaction(struct bt_hid_device_s *s,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     uint8_t type, parameter;
     int rlen, ret = -1;
@@ -362,7 +362,7 @@ static void bt_hid_control_transaction(struct bt_hid_device_s *s,
         bt_hid_send_handshake(s, ret);
 }
 
-static void bt_hid_control_sdu(void *opaque, const uint8_t *data, int len)
+static void bt_hid_control_sdu(void *opaque, const uint8_t *data, size_t len)
 {
     struct bt_hid_device_s *hid = opaque;
 
@@ -388,7 +388,7 @@ static void bt_hid_datain(HIDState *hs)
                         hid->datain.buffer, hid->datain.len);
 }
 
-static void bt_hid_interrupt_sdu(void *opaque, const uint8_t *data, int len)
+static void bt_hid_interrupt_sdu(void *opaque, const uint8_t *data, size_t len)
 {
     struct bt_hid_device_s *hid = opaque;
 
diff --git a/hw/bt/l2cap.c b/hw/bt/l2cap.c
index 9cf27f0df6..efd9a4b66a 100644
--- a/hw/bt/l2cap.c
+++ b/hw/bt/l2cap.c
@@ -32,10 +32,10 @@ struct l2cap_instance_s {
     int role;
 
     uint8_t frame_in[65535 + L2CAP_HDR_SIZE] __attribute__ ((aligned (4)));
-    int frame_in_len;
+    uint32_t frame_in_len;
 
     uint8_t frame_out[65535 + L2CAP_HDR_SIZE] __attribute__ ((aligned (4)));
-    int frame_out_len;
+    uint32_t frame_out_len;
 
     /* Signalling channel timers.  They exist per-request but we can make
      * sure we have no more than one outstanding request at any time.  */
@@ -49,7 +49,7 @@ struct l2cap_instance_s {
         struct bt_l2cap_conn_params_s params;
 
         void (*frame_in)(struct l2cap_chan_s *chan, uint16_t cid,
-                        const l2cap_hdr *hdr, int len);
+                        const l2cap_hdr *hdr, size_t len);
         int mps;
         int min_mtu;
 
@@ -68,7 +68,7 @@ struct l2cap_instance_s {
 
         /* Only flow-controlled, connection-oriented channels */
         uint8_t sdu[65536]; /* TODO: dynamically allocate */
-        int len_cur, len_total;
+        uint32_t len_cur, len_total;
         int rexmit;
         int monitor_timeout;
         QEMUTimer *monitor_timer;
@@ -140,7 +140,7 @@ static const uint16_t l2cap_fcs16_table[256] = {
     0x8201, 0x42c0, 0x4380, 0x8341, 0x4100, 0x81c1, 0x8081, 0x4040,
 };
 
-static uint16_t l2cap_fcs16(const uint8_t *message, int len)
+static uint16_t l2cap_fcs16(const uint8_t *message, size_t len)
 {
     uint16_t fcs = 0x0000;
 
@@ -186,7 +186,7 @@ static void l2cap_monitor_timer_update(struct l2cap_chan_s *ch)
 }
 
 static void l2cap_command_reject(struct l2cap_instance_s *l2cap, int id,
-                uint16_t reason, const void *data, int plen)
+                uint16_t reason, const void *data, size_t plen)
 {
     uint8_t *pkt;
     l2cap_cmd_hdr *hdr;
@@ -247,7 +247,7 @@ static void l2cap_connection_response(struct l2cap_instance_s *l2cap,
 }
 
 static void l2cap_configuration_request(struct l2cap_instance_s *l2cap,
-                int dcid, int flag, const uint8_t *data, int len)
+                int dcid, int flag, const uint8_t *data, size_t len)
 {
     uint8_t *pkt;
     l2cap_cmd_hdr *hdr;
@@ -275,7 +275,7 @@ static void l2cap_configuration_request(struct l2cap_instance_s *l2cap,
 }
 
 static void l2cap_configuration_response(struct l2cap_instance_s *l2cap,
-                int scid, int flag, int result, const uint8_t *data, int len)
+                int scid, int flag, int result, const uint8_t *data, size_t len)
 {
     uint8_t *pkt;
     l2cap_cmd_hdr *hdr;
@@ -322,7 +322,7 @@ static void l2cap_disconnection_response(struct l2cap_instance_s *l2cap,
 }
 
 static void l2cap_echo_response(struct l2cap_instance_s *l2cap,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     uint8_t *pkt;
     l2cap_cmd_hdr *hdr;
@@ -343,7 +343,7 @@ static void l2cap_echo_response(struct l2cap_instance_s *l2cap,
 }
 
 static void l2cap_info_response(struct l2cap_instance_s *l2cap, int type,
-                int result, const uint8_t *data, int len)
+                int result, const uint8_t *data, size_t len)
 {
     uint8_t *pkt;
     l2cap_cmd_hdr *hdr;
@@ -366,16 +366,18 @@ static void l2cap_info_response(struct l2cap_instance_s *l2cap, int type,
     l2cap->signalling_ch.params.sdu_submit(&l2cap->signalling_ch.params);
 }
 
-static uint8_t *l2cap_bframe_out(struct bt_l2cap_conn_params_s *parm, int len);
+static uint8_t *l2cap_bframe_out(struct bt_l2cap_conn_params_s *parm,
+                size_t len);
 static void l2cap_bframe_submit(struct bt_l2cap_conn_params_s *parms);
 #if 0
-static uint8_t *l2cap_iframe_out(struct bt_l2cap_conn_params_s *parm, int len);
+static uint8_t *l2cap_iframe_out(struct bt_l2cap_conn_params_s *parm,
+                size_t len);
 static void l2cap_iframe_submit(struct bt_l2cap_conn_params_s *parm);
 #endif
 static void l2cap_bframe_in(struct l2cap_chan_s *ch, uint16_t cid,
-                const l2cap_hdr *hdr, int len);
+                const l2cap_hdr *hdr, size_t len);
 static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,
-                const l2cap_hdr *hdr, int len);
+                const l2cap_hdr *hdr, size_t len);
 
 static int l2cap_cid_new(struct l2cap_instance_s *l2cap)
 {
@@ -499,7 +501,7 @@ static void l2cap_channel_config_req_event(struct l2cap_instance_s *l2cap,
 
 static int l2cap_channel_config(struct l2cap_instance_s *l2cap,
                 struct l2cap_chan_s *ch, int flag,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     l2cap_conf_opt *opt;
     l2cap_conf_opt_qos *qos;
@@ -684,7 +686,7 @@ static int l2cap_channel_config(struct l2cap_instance_s *l2cap,
 }
 
 static void l2cap_channel_config_req_msg(struct l2cap_instance_s *l2cap,
-                int flag, int cid, const uint8_t *data, int len)
+                int flag, int cid, const uint8_t *data, size_t len)
 {
     struct l2cap_chan_s *ch;
 
@@ -716,7 +718,7 @@ static void l2cap_channel_config_req_msg(struct l2cap_instance_s *l2cap,
 }
 
 static int l2cap_channel_config_rsp_msg(struct l2cap_instance_s *l2cap,
-                int result, int flag, int cid, const uint8_t *data, int len)
+                int result, int flag, int cid, const uint8_t *data, size_t len)
 {
     struct l2cap_chan_s *ch;
 
@@ -784,7 +786,7 @@ static void l2cap_info(struct l2cap_instance_s *l2cap, int type)
 }
 
 static void l2cap_command(struct l2cap_instance_s *l2cap, int code, int id,
-                const uint8_t *params, int len)
+                const uint8_t *params, size_t len)
 {
     int err;
 
@@ -939,7 +941,7 @@ static void l2cap_rexmit_enable(struct l2cap_chan_s *ch, int enable)
 }
 
 /* Command frame SDU */
-static void l2cap_cframe_in(void *opaque, const uint8_t *data, int len)
+static void l2cap_cframe_in(void *opaque, const uint8_t *data, size_t len)
 {
     struct l2cap_instance_s *l2cap = opaque;
     const l2cap_cmd_hdr *hdr;
@@ -967,7 +969,7 @@ static void l2cap_cframe_in(void *opaque, const uint8_t *data, int len)
 }
 
 /* Group frame SDU */
-static void l2cap_gframe_in(void *opaque, const uint8_t *data, int len)
+static void l2cap_gframe_in(void *opaque, const uint8_t *data, size_t len)
 {
 }
 
@@ -978,7 +980,7 @@ static void l2cap_sframe_in(struct l2cap_chan_s *ch, uint16_t ctrl)
 
 /* Basic L2CAP mode Information frame */
 static void l2cap_bframe_in(struct l2cap_chan_s *ch, uint16_t cid,
-                const l2cap_hdr *hdr, int len)
+                const l2cap_hdr *hdr, size_t len)
 {
     /* We have a full SDU, no further processing */
     ch->params.sdu_in(ch->params.opaque, hdr->data, len);
@@ -986,7 +988,7 @@ static void l2cap_bframe_in(struct l2cap_chan_s *ch, uint16_t cid,
 
 /* Flow Control and Retransmission mode frame */
 static void l2cap_iframe_in(struct l2cap_chan_s *ch, uint16_t cid,
-                const l2cap_hdr *hdr, int len)
+                const l2cap_hdr *hdr, size_t len)
 {
     uint16_t fcs = lduw_le_p(hdr->data + len - 2);
 
@@ -1077,7 +1079,7 @@ static void l2cap_frame_in(struct l2cap_instance_s *l2cap,
 
 /* "Recombination" */
 static void l2cap_pdu_in(struct l2cap_instance_s *l2cap,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     const l2cap_hdr *hdr = (void *) l2cap->frame_in;
 
@@ -1124,7 +1126,7 @@ static inline void l2cap_pdu_submit(struct l2cap_instance_s *l2cap)
             (l2cap->link, l2cap->frame_out, 1, l2cap->frame_out_len);
 }
 
-static uint8_t *l2cap_bframe_out(struct bt_l2cap_conn_params_s *parm, int len)
+static uint8_t *l2cap_bframe_out(struct bt_l2cap_conn_params_s *parm, size_t len)
 {
     struct l2cap_chan_s *chan = (struct l2cap_chan_s *) parm;
 
@@ -1147,7 +1149,7 @@ static void l2cap_bframe_submit(struct bt_l2cap_conn_params_s *parms)
 
 #if 0
 /* Stub: Only used if an emulated device requests outgoing flow control */
-static uint8_t *l2cap_iframe_out(struct bt_l2cap_conn_params_s *parm, int len)
+static uint8_t *l2cap_iframe_out(struct bt_l2cap_conn_params_s *parm, size_t len)
 {
     struct l2cap_chan_s *chan = (struct l2cap_chan_s *) parm;
 
@@ -1292,7 +1294,7 @@ static void l2cap_lmp_disconnect_slave(struct bt_link_s *link)
 }
 
 static void l2cap_lmp_acl_data_slave(struct bt_link_s *link,
-                const uint8_t *data, int start, int len)
+                const uint8_t *data, int start, size_t len)
 {
     struct slave_l2cap_instance_s *l2cap =
             (struct slave_l2cap_instance_s *) link;
@@ -1305,7 +1307,7 @@ static void l2cap_lmp_acl_data_slave(struct bt_link_s *link,
 
 /* Stub */
 static void l2cap_lmp_acl_data_host(struct bt_link_s *link,
-                const uint8_t *data, int start, int len)
+                const uint8_t *data, int start, size_t len)
 {
     struct bt_l2cap_device_s *dev = (struct bt_l2cap_device_s *) link->host;
     struct l2cap_instance_s *l2cap =
diff --git a/hw/bt/sdp.c b/hw/bt/sdp.c
index f4aba9d74f..163d315874 100644
--- a/hw/bt/sdp.c
+++ b/hw/bt/sdp.c
@@ -497,7 +497,7 @@ static ssize_t sdp_svc_search_attr_get(struct bt_l2cap_sdp_state_s *sdp,
     return end + 2;
 }
 
-static void bt_l2cap_sdp_sdu_in(void *opaque, const uint8_t *data, int len)
+static void bt_l2cap_sdp_sdu_in(void *opaque, const uint8_t *data, size_t len)
 {
     struct bt_l2cap_sdp_state_s *sdp = opaque;
     enum bt_sdp_cmd pdu_id;
@@ -507,7 +507,7 @@ static void bt_l2cap_sdp_sdu_in(void *opaque, const uint8_t *data, int len)
     int rsp_len = 0;
 
     if (len < 5) {
-        error_report("%s: short SDP PDU (%iB).", __func__, len);
+        error_report("%s: short SDP PDU (%zuB).", __func__, len);
         return;
     }
 
@@ -518,7 +518,7 @@ static void bt_l2cap_sdp_sdu_in(void *opaque, const uint8_t *data, int len)
     len -= 5;
 
     if (len != plen) {
-        error_report("%s: wrong SDP PDU length (%iB != %iB).",
+        error_report("%s: wrong SDP PDU length (%iB != %zuB).",
                         __func__, plen, len);
         err = SDP_INVALID_PDU_SIZE;
         goto respond;
diff --git a/hw/usb/dev-bluetooth.c b/hw/usb/dev-bluetooth.c
index eac7365b0a..40729210d2 100644
--- a/hw/usb/dev-bluetooth.c
+++ b/hw/usb/dev-bluetooth.c
@@ -319,7 +319,7 @@ static inline void usb_bt_fifo_dequeue(struct usb_hci_in_fifo_s *fifo,
 
 static inline void usb_bt_fifo_out_enqueue(struct USBBtState *s,
                 struct usb_hci_out_fifo_s *fifo,
-                void (*send)(struct HCIInfo *, const uint8_t *, int),
+                void (*send)(struct HCIInfo *, const uint8_t *, size_t),
                 int (*complete)(const uint8_t *, int),
                 USBPacket *p)
 {
@@ -478,7 +478,7 @@ static void usb_bt_handle_data(USBDevice *dev, USBPacket *p)
 }
 
 static void usb_bt_out_hci_packet_event(void *opaque,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     struct USBBtState *s = (struct USBBtState *) opaque;
 
@@ -489,7 +489,7 @@ static void usb_bt_out_hci_packet_event(void *opaque,
 }
 
 static void usb_bt_out_hci_packet_acl(void *opaque,
-                const uint8_t *data, int len)
+                const uint8_t *data, size_t len)
 {
     struct USBBtState *s = (struct USBBtState *) opaque;
 
diff --git a/include/hw/bt.h b/include/hw/bt.h
index b5e11d4d43..bc362aa662 100644
--- a/include/hw/bt.h
+++ b/include/hw/bt.h
@@ -94,9 +94,9 @@ struct bt_device_s {
     void (*lmp_disconnect_master)(struct bt_link_s *link);
     void (*lmp_disconnect_slave)(struct bt_link_s *link);
     void (*lmp_acl_data)(struct bt_link_s *link, const uint8_t *data,
-                    int start, int len);
+                    int start, size_t len);
     void (*lmp_acl_resp)(struct bt_link_s *link, const uint8_t *data,
-                    int start, int len);
+                    int start, size_t len);
     void (*lmp_mode_change)(struct bt_link_s *link);
 
     void (*handle_destroy)(struct bt_device_s *device);
@@ -148,12 +148,12 @@ struct bt_l2cap_device_s {
 
 struct bt_l2cap_conn_params_s {
     /* Input */
-    uint8_t *(*sdu_out)(struct bt_l2cap_conn_params_s *chan, int len);
+    uint8_t *(*sdu_out)(struct bt_l2cap_conn_params_s *chan, size_t len);
     void (*sdu_submit)(struct bt_l2cap_conn_params_s *chan);
     int remote_mtu;
     /* Output */
     void *opaque;
-    void (*sdu_in)(void *opaque, const uint8_t *data, int len);
+    void (*sdu_in)(void *opaque, const uint8_t *data, size_t len);
     void (*close)(void *opaque);
 };
 
diff --git a/include/sysemu/bt.h b/include/sysemu/bt.h
index ddb05cd109..db935c695d 100644
--- a/include/sysemu/bt.h
+++ b/include/sysemu/bt.h
@@ -5,12 +5,12 @@
 
 struct HCIInfo {
     int (*bdaddr_set)(struct HCIInfo *hci, const uint8_t *bd_addr);
-    void (*cmd_send)(struct HCIInfo *hci, const uint8_t *data, int len);
-    void (*sco_send)(struct HCIInfo *hci, const uint8_t *data, int len);
-    void (*acl_send)(struct HCIInfo *hci, const uint8_t *data, int len);
+    void (*cmd_send)(struct HCIInfo *hci, const uint8_t *data, size_t len);
+    void (*sco_send)(struct HCIInfo *hci, const uint8_t *data, size_t len);
+    void (*acl_send)(struct HCIInfo *hci, const uint8_t *data, size_t len);
     void *opaque;
-    void (*evt_recv)(void *opaque, const uint8_t *data, int len);
-    void (*acl_recv)(void *opaque, const uint8_t *data, int len);
+    void (*evt_recv)(void *opaque, const uint8_t *data, size_t len);
+    void (*acl_recv)(void *opaque, const uint8_t *data, size_t len);
 };
 
 /* bt-host.c */
-- 
2.17.2

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] bt: use size_t type for length parameters instead of signed int
  2018-10-17 20:58 [Qemu-devel] [PATCH] bt: use size_t type for length parameters instead of signed int P J P
@ 2018-10-18 10:59 ` Paolo Bonzini
  2018-10-18 12:01   ` P J P
  0 siblings, 1 reply; 3+ messages in thread
From: Paolo Bonzini @ 2018-10-18 10:59 UTC (permalink / raw)
  To: P J P, QEMU Developers; +Cc: Arash TC, Philippe Mathieu Daude, Prasad J Pandit

On 17/10/2018 22:58, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> The length parameter values are not negative, thus use an unsigned
> type 'size_t' for them. Many routines pass 'len' values to memcpy(3)
> calls. If it was negative, it could lead to memory corruption issues.

You are not fixing anything here; if the length was negative before, it
would still overflow and it would now be a huge positive value.

So you have to first find out all places where something is subtracted
from the length, and ensure it's okay or add assertions.

Then you have to check a much more important issue: places that use a
fixed-size buffer such as vhci_host_send should range check len first,
again with an assertion if needed.

Only then it makes sense to use size_t.

Paolo

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] bt: use size_t type for length parameters instead of signed int
  2018-10-18 10:59 ` Paolo Bonzini
@ 2018-10-18 12:01   ` P J P
  0 siblings, 0 replies; 3+ messages in thread
From: P J P @ 2018-10-18 12:01 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: QEMU Developers, Arash TC, Philippe Mathieu Daude

+-- On Thu, 18 Oct 2018, Paolo Bonzini wrote --+
| So you have to first find out all places where something is subtracted
| from the length, and ensure it's okay or add assertions.
| 
| Then you have to check a much more important issue: places that use a 
| fixed-size buffer such as vhci_host_send should range check len first, again 
| with an assertion if needed.

  Okay. There are routines which range check 'len', checking others.

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-10-18 12:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-17 20:58 [Qemu-devel] [PATCH] bt: use size_t type for length parameters instead of signed int P J P
2018-10-18 10:59 ` Paolo Bonzini
2018-10-18 12:01   ` P J P

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.