[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 16/31] bt: rewrite csrhci_write to avoid out-of-bound
From: |
Paolo Bonzini |
Subject: |
[Qemu-devel] [PULL 16/31] bt: rewrite csrhci_write to avoid out-of-bounds writes |
Date: |
Fri, 27 May 2016 12:06:29 +0200 |
The usage of INT_MAX in this function confuses Coverity. I think
the defect is bogus, however there is no protection against
getting more than sizeof(s->inpkt) bytes from the character device
backend.
Rewrite the function to only fill in as much data as needed from
buf into s->inpkt. The plen variable is replaced by a simple
state machine and there is no need anymore to shift contents to
the beginning of s->inpkt.
Signed-off-by: Paolo Bonzini <address@hidden>
---
hw/bt/hci-csr.c | 67 +++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 46 insertions(+), 21 deletions(-)
diff --git a/hw/bt/hci-csr.c b/hw/bt/hci-csr.c
index e6b8998..d688372 100644
--- a/hw/bt/hci-csr.c
+++ b/hw/bt/hci-csr.c
@@ -39,9 +39,14 @@ struct csrhci_s {
int out_size;
uint8_t outfifo[FIFO_LEN * 2];
uint8_t inpkt[FIFO_LEN];
+ enum {
+ CSR_HDR_LEN,
+ CSR_DATA_LEN,
+ CSR_DATA
+ } in_state;
int in_len;
int in_hdr;
- int in_data;
+ int in_needed;
QEMUTimer *out_tm;
int64_t baud_delay;
@@ -296,38 +301,60 @@ static int csrhci_data_len(const uint8_t *pkt)
exit(-1);
}
+static void csrhci_ready_for_next_inpkt(struct csrhci_s *s)
+{
+ s->in_state = CSR_HDR_LEN;
+ s->in_len = 0;
+ s->in_needed = 2;
+ s->in_hdr = INT_MAX;
+}
+
static int csrhci_write(struct CharDriverState *chr,
const uint8_t *buf, int len)
{
struct csrhci_s *s = (struct csrhci_s *) chr->opaque;
- int plen = s->in_len;
+ int total = 0;
if (!s->enable)
return 0;
- s->in_len += len;
- memcpy(s->inpkt + plen, buf, len);
+ for (;;) {
+ int cnt = MIN(len, s->in_needed - s->in_len);
+ if (cnt) {
+ memcpy(s->inpkt + s->in_len, buf, cnt);
+ s->in_len += cnt;
+ buf += cnt;
+ len -= cnt;
+ total += cnt;
+ }
+
+ if (s->in_len < s->in_needed) {
+ break;
+ }
- while (1) {
- if (s->in_len >= 2 && plen < 2)
+ if (s->in_state == CSR_HDR_LEN) {
s->in_hdr = csrhci_header_len(s->inpkt) + 1;
+ assert(s->in_hdr >= s->in_needed);
+ s->in_needed = s->in_hdr;
+ s->in_state = CSR_DATA_LEN;
+ continue;
+ }
- if (s->in_len >= s->in_hdr && plen < s->in_hdr)
- s->in_data = csrhci_data_len(s->inpkt) + s->in_hdr;
+ if (s->in_state == CSR_DATA_LEN) {
+ s->in_needed += csrhci_data_len(s->inpkt);
+ /* hci_acl_hdr could specify more than 4096 bytes, so assert. */
+ assert(s->in_needed <= sizeof(s->inpkt));
+ s->in_state = CSR_DATA;
+ continue;
+ }
- if (s->in_len >= s->in_data) {
+ if (s->in_state == CSR_DATA) {
csrhci_in_packet(s, s->inpkt);
-
- memmove(s->inpkt, s->inpkt + s->in_len, s->in_len - s->in_data);
- s->in_len -= s->in_data;
- s->in_hdr = INT_MAX;
- s->in_data = INT_MAX;
- plen = 0;
- } else
- break;
+ csrhci_ready_for_next_inpkt(s);
+ }
}
- return len;
+ return total;
}
static void csrhci_out_hci_packet_event(void *opaque,
@@ -389,11 +416,9 @@ static void csrhci_reset(struct csrhci_s *s)
{
s->out_len = 0;
s->out_size = FIFO_LEN;
- s->in_len = 0;
+ csrhci_ready_for_next_inpkt(s);
s->baud_delay = NANOSECONDS_PER_SECOND;
s->enable = 0;
- s->in_hdr = INT_MAX;
- s->in_data = INT_MAX;
s->modem_state = 0;
/* After a while... (but sooner than 10ms) */
--
2.5.5
- [Qemu-devel] [PULL 11/31] scsi: mptsas: infinite loop while fetching requests, (continued)
- [Qemu-devel] [PULL 11/31] scsi: mptsas: infinite loop while fetching requests, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 13/31] scsi: megasas: initialise local configuration data buffer, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 14/31] scsi: megasas: check 'read_queue_head' index value, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 10/31] scsi: pvscsi: check command descriptor ring buffer size (CVE-2016-4952), Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 09/31] kvm_stat: Remove, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 12/31] scsi: megasas: use appropriate property buffer size, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 15/31] block/iscsi: avoid potential overflow of acb->task->cdb, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 18/31] atomics: emit an smp_read_barrier_depends() barrier only for Alpha and Thread Sanitizer, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 20/31] docs/atomics: update comparison with Linux, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 17/31] docs/atomics: update atomic_read/set comparison with Linux, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 16/31] bt: rewrite csrhci_write to avoid out-of-bounds writes,
Paolo Bonzini <=
- [Qemu-devel] [PULL 24/31] scsi-disk: add need_fua_emulation to SCSIDiskClass, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 19/31] atomics: do not emit consume barrier for atomic_rcu_read, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 21/31] xen-hvm: ignore background I/O sections, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 22/31] scsi-disk: introduce a common base class, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 25/31] scsi-disk: introduce scsi_disk_req_check_error, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 26/31] scsi-block: always use SG_IO, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 23/31] scsi-disk: introduce dma_readv and dma_writev, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 27/31] scsi-generic: Merge block max xfer len in INQUIRY response, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 31/31] exec: hide mr->ram_addr from qemu_get_ram_ptr users, Paolo Bonzini, 2016/05/27
- [Qemu-devel] [PULL 29/31] exec: remove ram_addr argument from qemu_ram_block_from_host, Paolo Bonzini, 2016/05/27