diff mbox

[10/25] nbd: Fix potential signed overflow issues

Message ID 1424887718-10800-11-git-send-email-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Feb. 25, 2015, 6:08 p.m. UTC
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 include/block/nbd.h | 4 ++--
 qemu-nbd.c          | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

Comments

Paolo Bonzini March 11, 2015, 11:28 a.m. UTC | #1
On 25/02/2015 19:08, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  include/block/nbd.h | 4 ++--
>  qemu-nbd.c          | 5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/block/nbd.h b/include/block/nbd.h
> index 2c20138..53726e8 100644
> --- a/include/block/nbd.h
> +++ b/include/block/nbd.h
> @@ -54,8 +54,8 @@ struct nbd_reply {
>  /* Reply types. */
>  #define NBD_REP_ACK             (1)             /* Data sending finished. */
>  #define NBD_REP_SERVER          (2)             /* Export description. */
> -#define NBD_REP_ERR_UNSUP       ((1 << 31) | 1) /* Unknown option. */
> -#define NBD_REP_ERR_INVALID     ((1 << 31) | 3) /* Invalid length. */
> +#define NBD_REP_ERR_UNSUP       ((UINT32_C(1) << 31) | 1) /* Unknown option. */
> +#define NBD_REP_ERR_INVALID     ((UINT32_C(1) << 31) | 3) /* Invalid length. */

Easier to just use 0x80000001u and 0x80000003u; changed locally.

>  
>  #define NBD_CMD_MASK_COMMAND	0x0000ffff
>  #define NBD_CMD_FLAG_FUA	(1 << 16)
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index c9ed003..fd1e0c8 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -142,8 +142,9 @@ static void read_partition(uint8_t *p, struct partition_record *r)
>      r->end_head = p[5];
>      r->end_cylinder = p[7] | ((p[6] << 2) & 0x300);
>      r->end_sector = p[6] & 0x3f;
> -    r->start_sector_abs = p[8] | p[9] << 8 | p[10] << 16 | p[11] << 24;
> -    r->nb_sectors_abs = p[12] | p[13] << 8 | p[14] << 16 | p[15] << 24;
> +
> +    r->start_sector_abs = le32_to_cpup((uint32_t *)(p +  8));
> +    r->nb_sectors_abs   = le32_to_cpup((uint32_t *)(p + 12));

By accepting uint32_t*, le32_to_cpup  is not safe if p is not properly
aligned.  ldl_le_p is better in this case.

Paolo

>  }
>  
>  static int find_partition(BlockBackend *blk, int partition,
>
diff mbox

Patch

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 2c20138..53726e8 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -54,8 +54,8 @@  struct nbd_reply {
 /* Reply types. */
 #define NBD_REP_ACK             (1)             /* Data sending finished. */
 #define NBD_REP_SERVER          (2)             /* Export description. */
-#define NBD_REP_ERR_UNSUP       ((1 << 31) | 1) /* Unknown option. */
-#define NBD_REP_ERR_INVALID     ((1 << 31) | 3) /* Invalid length. */
+#define NBD_REP_ERR_UNSUP       ((UINT32_C(1) << 31) | 1) /* Unknown option. */
+#define NBD_REP_ERR_INVALID     ((UINT32_C(1) << 31) | 3) /* Invalid length. */
 
 #define NBD_CMD_MASK_COMMAND	0x0000ffff
 #define NBD_CMD_FLAG_FUA	(1 << 16)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index c9ed003..fd1e0c8 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -142,8 +142,9 @@  static void read_partition(uint8_t *p, struct partition_record *r)
     r->end_head = p[5];
     r->end_cylinder = p[7] | ((p[6] << 2) & 0x300);
     r->end_sector = p[6] & 0x3f;
-    r->start_sector_abs = p[8] | p[9] << 8 | p[10] << 16 | p[11] << 24;
-    r->nb_sectors_abs = p[12] | p[13] << 8 | p[14] << 16 | p[15] << 24;
+
+    r->start_sector_abs = le32_to_cpup((uint32_t *)(p +  8));
+    r->nb_sectors_abs   = le32_to_cpup((uint32_t *)(p + 12));
 }
 
 static int find_partition(BlockBackend *blk, int partition,