hd-geo-test: Clean up use of buf[] in create_qcow2_with_mbr()

[PATCH] hd-geo-test: Clean up use of buf[] in create_qcow2_with_mbr()

Posted by Markus Armbruster 5 years, 10 months ago

valgrind reports write unitialized bytes from buf[].  Clear them.

ASan reports we store to misaligned address in buf[].  Use stl_le_p()
for that.

Cc: Sam Eiderman <shmuel.eiderman@oracle.com>
Cc: John Snow <jsnow@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 tests/qtest/hd-geo-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index a249800544..48e8e02d6e 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -421,7 +421,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
     char *raw_path = strdup(template);
     char *qcow2_path = strdup(template);
     char cmd[100 + 2 * PATH_MAX];
-    uint8_t buf[512];
+    uint8_t buf[512] = {};
     int i, ret, fd, offset;
     uint64_t qcow2_size = sectors * 512;
     uint8_t status, parttype, head, sector, cyl;
@@ -457,8 +457,8 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
         buf[offset + 0x6] = sector;
         buf[offset + 0x7] = cyl;
 
-        (*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
-        (*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
+        stl_le_p(&buf[offset + 0x8], mbr[i].start_sect);
+        stl_le_p(&buf[offset + 0xc], mbr[i].nr_sects);
 
         offset += 0x10;
     }
-- 
2.21.1

Re: [PATCH] hd-geo-test: Clean up use of buf[] in create_qcow2_with_mbr()

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

On 3/17/20 10:23 AM, Markus Armbruster wrote:
> valgrind reports write unitialized bytes from buf[].  Clear them.
> 
> ASan reports we store to misaligned address in buf[].  Use stl_le_p()
> for that.
> 
> Cc: Sam Eiderman <shmuel.eiderman@oracle.com>
> Cc: John Snow <jsnow@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   tests/qtest/hd-geo-test.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index a249800544..48e8e02d6e 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -421,7 +421,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
>       char *raw_path = strdup(template);
>       char *qcow2_path = strdup(template);
>       char cmd[100 + 2 * PATH_MAX];
> -    uint8_t buf[512];
> +    uint8_t buf[512] = {};
>       int i, ret, fd, offset;
>       uint64_t qcow2_size = sectors * 512;
>       uint8_t status, parttype, head, sector, cyl;
> @@ -457,8 +457,8 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
>           buf[offset + 0x6] = sector;
>           buf[offset + 0x7] = cyl;
>   
> -        (*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
> -        (*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
> +        stl_le_p(&buf[offset + 0x8], mbr[i].start_sect);
> +        stl_le_p(&buf[offset + 0xc], mbr[i].nr_sects);
>   
>           offset += 0x10;
>       }
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH] hd-geo-test: Clean up use of buf[] in create_qcow2_with_mbr()

Posted by John Snow 5 years, 10 months ago


On 3/17/20 5:23 AM, Markus Armbruster wrote:
> valgrind reports write unitialized bytes from buf[].  Clear them.
> 
> ASan reports we store to misaligned address in buf[].  Use stl_le_p()
> for that.
> 
> Cc: Sam Eiderman <shmuel.eiderman@oracle.com>
> Cc: John Snow <jsnow@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  tests/qtest/hd-geo-test.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index a249800544..48e8e02d6e 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -421,7 +421,7 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
>      char *raw_path = strdup(template);
>      char *qcow2_path = strdup(template);
>      char cmd[100 + 2 * PATH_MAX];
> -    uint8_t buf[512];
> +    uint8_t buf[512] = {};
>      int i, ret, fd, offset;
>      uint64_t qcow2_size = sectors * 512;
>      uint8_t status, parttype, head, sector, cyl;
> @@ -457,8 +457,8 @@ static char *create_qcow2_with_mbr(MBRpartitions mbr, uint64_t sectors)
>          buf[offset + 0x6] = sector;
>          buf[offset + 0x7] = cyl;
>  
> -        (*(uint32_t *)&buf[offset + 0x8]) = cpu_to_le32(mbr[i].start_sect);
> -        (*(uint32_t *)&buf[offset + 0xc]) = cpu_to_le32(mbr[i].nr_sects);
> +        stl_le_p(&buf[offset + 0x8], mbr[i].start_sect);
> +        stl_le_p(&buf[offset + 0xc], mbr[i].nr_sects);
>  
>          offset += 0x10;
>      }
> 

Staged.

Trying out our gitlab CI pipelines now:

https://gitlab.com/jsnow/qemu/-/tree/ide


Traditional message:

Thanks, applied to my IDE tree:

https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git

--js