hw/i2c/microbit_i2c: Don't index off end of twi_read_sequence[]

[PATCH] hw/i2c/microbit_i2c: Don't index off end of twi_read_sequence[]

Posted by Peter Maydell 4 weeks, 1 day ago

If the guest tries to read more bytes from our fake stub I2C device
than we have provided, we incorrectly read one byte beyond the end of
this array. Avoid this, and instead keep reporting the RXD register
as containing the last byte of the "data transfer".

Cc: qemu-stable@nongnu.org
Fixes: 9d68bf564ec ("arm: Stub out NRF51 TWI magnetometer/accelerometer detection")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3408
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/i2c/microbit_i2c.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/i2c/microbit_i2c.c b/hw/i2c/microbit_i2c.c
index 2291d6370e..d9689b6f1a 100644
--- a/hw/i2c/microbit_i2c.c
+++ b/hw/i2c/microbit_i2c.c
@@ -41,8 +41,13 @@ static uint64_t microbit_i2c_read(void *opaque, hwaddr addr, unsigned int size)
         data = 0x01;
         break;
     case NRF51_TWI_REG_RXD:
+        /*
+         * Return the next byte from our fake data sequence. If
+         * the guest keeps reading the register after that, keep
+         * returning the same last byte value.
+         */
         data = twi_read_sequence[s->read_idx];
-        if (s->read_idx < G_N_ELEMENTS(twi_read_sequence)) {
+        if (s->read_idx + 1 < G_N_ELEMENTS(twi_read_sequence)) {
             s->read_idx++;
         }
         break;
-- 
2.43.0

Re: [PATCH] hw/i2c/microbit_i2c: Don't index off end of twi_read_sequence[]

Posted by Peter Maydell 2 weeks, 5 days ago

Hi; ping for review?

thanks
-- PMM

On Fri, 1 May 2026 at 17:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> If the guest tries to read more bytes from our fake stub I2C device
> than we have provided, we incorrectly read one byte beyond the end of
> this array. Avoid this, and instead keep reporting the RXD register
> as containing the last byte of the "data transfer".
>
> Cc: qemu-stable@nongnu.org
> Fixes: 9d68bf564ec ("arm: Stub out NRF51 TWI magnetometer/accelerometer detection")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3408
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/i2c/microbit_i2c.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/i2c/microbit_i2c.c b/hw/i2c/microbit_i2c.c
> index 2291d6370e..d9689b6f1a 100644
> --- a/hw/i2c/microbit_i2c.c
> +++ b/hw/i2c/microbit_i2c.c
> @@ -41,8 +41,13 @@ static uint64_t microbit_i2c_read(void *opaque, hwaddr addr, unsigned int size)
>          data = 0x01;
>          break;
>      case NRF51_TWI_REG_RXD:
> +        /*
> +         * Return the next byte from our fake data sequence. If
> +         * the guest keeps reading the register after that, keep
> +         * returning the same last byte value.
> +         */
>          data = twi_read_sequence[s->read_idx];
> -        if (s->read_idx < G_N_ELEMENTS(twi_read_sequence)) {
> +        if (s->read_idx + 1 < G_N_ELEMENTS(twi_read_sequence)) {
>              s->read_idx++;
>          }
>          break;
> --

Re: [PATCH] hw/i2c/microbit_i2c: Don't index off end of twi_read_sequence[]

Posted by Philippe Mathieu-Daudé 2 weeks, 4 days ago

On 11/5/26 16:48, Peter Maydell wrote:
> Hi; ping for review?
> 
> thanks
> -- PMM
> 
> On Fri, 1 May 2026 at 17:26, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> If the guest tries to read more bytes from our fake stub I2C device
>> than we have provided, we incorrectly read one byte beyond the end of
>> this array. Avoid this, and instead keep reporting the RXD register
>> as containing the last byte of the "data transfer".

Having the accelerator data buried in an i2c controller is not good
practice. Anyway good enough for how we use it. Same for this fix:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 9d68bf564ec ("arm: Stub out NRF51 TWI magnetometer/accelerometer detection")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3408
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>   hw/i2c/microbit_i2c.c | 7 ++++++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i2c/microbit_i2c.c b/hw/i2c/microbit_i2c.c
>> index 2291d6370e..d9689b6f1a 100644
>> --- a/hw/i2c/microbit_i2c.c
>> +++ b/hw/i2c/microbit_i2c.c
>> @@ -41,8 +41,13 @@ static uint64_t microbit_i2c_read(void *opaque, hwaddr addr, unsigned int size)
>>           data = 0x01;
>>           break;
>>       case NRF51_TWI_REG_RXD:
>> +        /*
>> +         * Return the next byte from our fake data sequence. If
>> +         * the guest keeps reading the register after that, keep
>> +         * returning the same last byte value.
>> +         */
>>           data = twi_read_sequence[s->read_idx];
>> -        if (s->read_idx < G_N_ELEMENTS(twi_read_sequence)) {
>> +        if (s->read_idx + 1 < G_N_ELEMENTS(twi_read_sequence)) {
>>               s->read_idx++;
>>           }
>>           break;
>> --
>