[Qemu-devel] [PATCH 1/1] tmp105: Fix I2C protocol bug

[Alex Horn]

The private buffer length field must only be incremented after the I2C
frame has been transmitted.

To expose this bug, assume the temperature in the TMP105 hardware model
is +0.125 C (e.g. snow slush). Note that eleven bit precision is required
to read this value; otherwise the reading is equal to zero centigrade (ice).

Continue by considering the following I2C protocol steps:

1) Start transfer with I2C_START_SEND
2) Send byte 0x01 (i.e. configuration register)
3) Send byte 0x40 (i.e. eleven bit precision)
4) End transfer with I2C_FINISH

5) Start transfer with I2C_START_SEND
6) Send byte 0x00 (i.e. temperature register)
7) End transfer I2C_FINISH

8) Start transfer with I2C_START_RECV
9) Receive high-order byte of temperature
   ...

In step (1), the function tmp105_tx() is called. By the conditional
check !s->len and the side effect with ++, s->len is equal to 1 when
step (2) begins. Thus, 0x40 is written to s->buf[1] in step (3).
By definition of tmp105_write(), s->config is set to zero in step (3).
Thus, when we read the higher-order byte in step (9), it is zero!

In other words, the TMP105 hardware model allows us to measure 0 C (ice)
even with eleven bit precision when, in fact, it should be 0.125 C (slush)!

Signed-off-by: Alex Horn <address@hidden>
---
 hw/tmp105.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/hw/tmp105.c b/hw/tmp105.c
index 8e8dbd9..5f41a3f 100644
--- a/hw/tmp105.c
+++ b/hw/tmp105.c
@@ -152,7 +152,7 @@ static int tmp105_tx(I2CSlave *i2c, uint8_t data)
 {
     TMP105State *s = (TMP105State *) i2c;
 
-    if (!s->len ++)
+    if (s->len == 0)
         s->pointer = data;
     else {
         if (s->len <= 2)
@@ -160,6 +160,7 @@ static int tmp105_tx(I2CSlave *i2c, uint8_t data)
         tmp105_write(s);
     }
 
+    s->len ++;
     return 0;
 }
 
-- 
1.7.6.5