From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48705) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YVeow-00009b-Sr for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:29:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YVeot-0008Du-O9 for qemu-devel@nongnu.org; Wed, 11 Mar 2015 07:29:02 -0400 Sender: Paolo Bonzini Message-ID: <550026F6.6090304@redhat.com> Date: Wed, 11 Mar 2015 12:28:54 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1424887718-10800-1-git-send-email-mreitz@redhat.com> <1424887718-10800-11-git-send-email-mreitz@redhat.com> In-Reply-To: <1424887718-10800-11-git-send-email-mreitz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 10/25] nbd: Fix potential signed overflow issues List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz , qemu-block@nongnu.org Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On 25/02/2015 19:08, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > 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, >