Skip to contentSkip to conversation

i2c: pm_smbus: check smb_index before block transfer write

PATCH Applied P J P 4 participants 4 messages Dec 6, 2018

lore
Share & tools
lore

P J P

author
#0 lore raw

PATCH 1/1 i2c: pm_smbus: check smb_index before block transfer write

From: Prasad J Pandit <[email protected]>

While performing block transfer write in smb_ioport_writeb(), 'smb_index' is incremented and used to index smb_data[] array. Check 'smb_index' value to avoid OOB access.

Note that this bug is exploitable by a guest to escape from the virtual machine. However the commit which introduced the bug was only made after the 3.0 release, and so it is not present in any released QEMU versions.

Fixes 38ad4fae43 i2c: pm_smbus: Add block transfer capability
Reported-by Michael Hanselmann <[email protected]>
Signed-off-by Prasad J Pandit <[email protected]>
---
 hw/i2c/pm_smbus.c | 3 +++
 1 file changed, 3 insertions(+)

Update v1: add note about issue being introduced after 3.0 release
-> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01115.html

hw/i2c/pm_smbus.c +3 -0
index 685a2378ed..03062740cc 100644
@@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
             uint8_t read = s->smb_addr & 0x01;
 
             s->smb_index++;
+            if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) {
+                s->smb_index = 0;
+            }
             if (!read && s->smb_index == s->smb_data0) {
                 uint8_t prot = (s->smb_ctl >> 2) & 0x07;
                 uint8_t cmd = s->smb_cmd;
Signature · 1 line
2.19.2
Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · P J P

From: Prasad J Pandit <[email protected]>

While performing block transfer write in smb_ioport_writeb(), 'smb_index' is incremented and used to index smb_data[] array. Check 'smb_index' value to avoid OOB access.

Quoted 8 lines

Note that this bug is exploitable by a guest to escape from the virtual machine. However the commit which introduced the bug was only made after the 3.0 release, and so it is not present in any released QEMU versions.

Fixes 38ad4fae43 i2c: pm_smbus: Add block transfer capability
Reported-by Michael Hanselmann <[email protected]>
Signed-off-by Prasad J Pandit <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Quoted 22 lines
---
 hw/i2c/pm_smbus.c | 3 +++
 1 file changed, 3 insertions(+)

Update v1: add note about issue being introduced after 3.0 release
-> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01115.html

hw/i2c/pm_smbus.c +3 -0
index 685a2378ed..03062740cc 100644
@@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
             uint8_t read = s->smb_addr & 0x01;
 
             s->smb_index++;
+            if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) {
+                s->smb_index = 0;
+            }
             if (!read && s->smb_index == s->smb_data0) {
                 uint8_t prot = (s->smb_ctl >> 2) & 0x07;
                 uint8_t cmd = s->smb_cmd;
Reviewed-by on v1 1/1
#2 lore raw
In reply to P J P
Quoted 6 lines · P J P

From: Prasad J Pandit <[email protected]>

While performing block transfer write in smb_ioport_writeb(), 'smb_index' is incremented and used to index smb_data[] array. Check 'smb_index' value to avoid OOB access.

Quoted 8 lines

Note that this bug is exploitable by a guest to escape from the virtual machine. However the commit which introduced the bug was only made after the 3.0 release, and so it is not present in any released QEMU versions.

Fixes 38ad4fae43 i2c: pm_smbus: Add block transfer capability
Reported-by Michael Hanselmann <[email protected]>
Signed-off-by Prasad J Pandit <[email protected]>
Reviewed-by Michael S. Tsirkin <[email protected]>
Quoted 12 lines
---
 hw/i2c/pm_smbus.c | 3 +++
 1 file changed, 3 insertions(+)

Update v1: add note about issue being introduced after 3.0 release
-> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01115.html

hw/i2c/pm_smbus.c +0 -0
index 685a2378ed..03062740cc 100644

hw/i2c/pm_smbus.c static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val, · line 245

Quoted 6 lines
hw/i2c/pm_smbus.c +3 -0
@@ -240,6 +240,9 @@ static void smb_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
             uint8_t read = s->smb_addr & 0x01;
 
             s->smb_index++;
+            if (s->smb_index >= PM_SMBUS_MAX_MSG_SIZE) {
+                s->smb_index = 0;
+            }

when I see patterns like these I always think "what about spectre". But not a new issue and not the only place, so we can leave this for another day.

Quoted 5 lines
hw/i2c/pm_smbus.c +0 -0
             if (!read && s->smb_index == s->smb_data0) {
                 uint8_t prot = (s->smb_ctl >> 2) & 0x07;
                 uint8_t cmd = s->smb_cmd;
Signature · 1 line
2.19.2
Applied
#3 lore raw
In reply to P J P
Quoted 15 lines · P J P

From: Prasad J Pandit <[email protected]>

While performing block transfer write in smb_ioport_writeb(), 'smb_index' is incremented and used to index smb_data[] array. Check 'smb_index' value to avoid OOB access.

Note that this bug is exploitable by a guest to escape from the virtual machine. However the commit which introduced the bug was only made after the 3.0 release, and so it is not present in any released QEMU versions.

Fixes 38ad4fae43 i2c: pm_smbus: Add block transfer capability
Reported-by Michael Hanselmann <[email protected]>
Signed-off-by Prasad J Pandit <[email protected]>
Quoted 6 lines
---
 hw/i2c/pm_smbus.c | 3 +++
 1 file changed, 3 insertions(+)

Update v1: add note about issue being introduced after 3.0 release
-> https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg01115.html

Applied, thanks.

-- PMM