On Fri, Jun 1, 2018 at 11:41 AM, Stefan Hajnoczi <address@hidden> wrote:
On Thu, May 31, 2018 at 2:58 PM, sundeep subbaraya
<address@hidden> wrote:
On Wed, May 30, 2018 at 3:33 AM, Julia Suvorova via Qemu-devel
<address@hidden> wrote:
+static uint64_t uart_read(void *opaque, hwaddr addr, unsigned int size)
+{
+ Nrf51UART *s = NRF51_UART(opaque);
+ uint64_t r;
+
+ switch (addr) {
+ case A_RXD:
+ r = s->rx_fifo[s->rx_fifo_pos];
+ if (s->rx_fifo_len > 0) {
+ s->rx_fifo_pos = (s->rx_fifo_pos + 1) % UART_FIFO_LENGTH;
+ s->rx_fifo_len--;
+ qemu_chr_fe_accept_input(&s->chr);
+ }
+ break;
+
+ case A_INTENSET:
+ case A_INTENCLR:
+ case A_INTEN:
+ r = s->reg[A_INTEN];
+ break;
+ default:
+ r = s->reg[addr];
You can use R_* macros for registers and access regs[ ] with addr/4 as index.
It is better than using big regs[ ] array out of which most of
locations go unused.
Good point. The bug is more severe than an inefficiency.
s->reg[addr] allows out-of-bounds accesses. This is a security bug.
The memory region is 0x1000 *bytes* long, but the array has 0x1000
32-bit *elements*. A read from address 0xfffc results in a memory
load from s->reg + 0xfffc * sizeof(s->reg[0]). That's beyond the end
of the array!
Sorry, I was wrong. The array is large enough after all. It's just
an inefficiency, but still worth fixing. Similar issues could lead to
out-of-bound accesses.