diff mbox series

[1/2] sd: check bit number before setting card_status flag

Message ID 20200520152450.200362-2-ppandit@redhat.com
State New
Headers show
Series avoid OOB access in SD card emulator | expand

Commit Message

Prasad Pandit May 20, 2020, 3:24 p.m. UTC
From: Prasad J Pandit <pjp@fedoraproject.org>

SD card emulator sets 'sd->card_status' while performing block
write commands. While doing so, it tests the corresponding bit
derived from 's->data_start' address. This may lead to OOB access.
Add check to avoid it.

Reported-by: Alex <alxndr@bu.edu>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
---
 hw/sd/sd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé May 20, 2020, 4:40 p.m. UTC | #1
Hi Prasad,

On 5/20/20 5:24 PM, P J P wrote:
> From: Prasad J Pandit <pjp@fedoraproject.org>
> 
> SD card emulator sets 'sd->card_status' while performing block
> write commands. While doing so, it tests the corresponding bit
> derived from 's->data_start' address. This may lead to OOB access.
> Add check to avoid it.

Ah, this is different that the one reported recently:
https://bugs.launchpad.net/qemu/+bug/1878054

Do you have a reproducer?

Is this a CVE?

> 
> Reported-by: Alex <alxndr@bu.edu>

This is not Alexander complete name.

> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
> ---
>   hw/sd/sd.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 71a9af09ab..916e9fff58 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -802,7 +802,12 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
>   
>   static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
>   {
> -    return test_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
> +    uint64_t bit = sd_addr_to_wpnum(addr);
> +
> +    if (bit < sd->wpgrps_size) {

This should never be called with a such address, so I'd simply use an 
assertion here.

The problem is earlier where the address should be validated and a 
protocol error returned.

> +        return test_bit(bit, sd->wp_groups);
> +    }
> +    return true;
>   }
>   
>   static void sd_lock_command(SDState *sd)
>
diff mbox series

Patch

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 71a9af09ab..916e9fff58 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -802,7 +802,12 @@  static void sd_function_switch(SDState *sd, uint32_t arg)
 
 static inline bool sd_wp_addr(SDState *sd, uint64_t addr)
 {
-    return test_bit(sd_addr_to_wpnum(addr), sd->wp_groups);
+    uint64_t bit = sd_addr_to_wpnum(addr);
+
+    if (bit < sd->wpgrps_size) {
+        return test_bit(bit, sd->wp_groups);
+    }
+    return true;
 }
 
 static void sd_lock_command(SDState *sd)