[v7] hw/sd/sdcard: Fix CVE-2020-13253 & cleanups

[PATCH v7 08/17] hw/sd/sdcard: Call sd_addr_to_wpnum where it is used, consider zero size

Posted by Philippe Mathieu-Daudé 5 years, 7 months ago

Avoid setting 'sect' variable just once (its name is confuse
anyway). Directly set 'sd->wpgrps_size'. Special case when
size is zero.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 078b0e81ee..e5adcc8055 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -556,8 +556,6 @@ static void sd_reset(DeviceState *dev)
     }
     size = sect << 9;
 
-    sect = sd_addr_to_wpnum(size) + 1;
-
     sd->size = size;
     sd->state = sd_idle_state;
     sd->rca = 0x0000;
@@ -570,7 +568,11 @@ static void sd_reset(DeviceState *dev)
 
     g_free(sd->wp_groups);
     sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false;
-    sd->wpgrps_size = sect;
+    if (sd->size) {
+        sd->wpgrps_size = sd_addr_to_wpnum(sd, sd->size) + 1;
+    } else {
+        sd->wpgrps_size = 1;
+    }
     sd->wp_groups = bitmap_new(sd->wpgrps_size);
     memset(sd->function_group, 0, sizeof(sd->function_group));
     sd->erase_start = 0;
-- 
2.21.3

Re: [PATCH v7 08/17] hw/sd/sdcard: Call sd_addr_to_wpnum where it is used, consider zero size

Posted by Peter Maydell 5 years, 7 months ago

On Tue, 30 Jun 2020 at 14:39, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Avoid setting 'sect' variable just once (its name is confuse
> anyway). Directly set 'sd->wpgrps_size'. Special case when
> size is zero.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/sd.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 078b0e81ee..e5adcc8055 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -556,8 +556,6 @@ static void sd_reset(DeviceState *dev)
>      }
>      size = sect << 9;
>
> -    sect = sd_addr_to_wpnum(size) + 1;
> -
>      sd->size = size;
>      sd->state = sd_idle_state;
>      sd->rca = 0x0000;
> @@ -570,7 +568,11 @@ static void sd_reset(DeviceState *dev)
>
>      g_free(sd->wp_groups);
>      sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false;
> -    sd->wpgrps_size = sect;
> +    if (sd->size) {
> +        sd->wpgrps_size = sd_addr_to_wpnum(sd, sd->size) + 1;
> +    } else {
> +        sd->wpgrps_size = 1;
> +    }

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM