From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40183) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1abRGR-0002aL-PU for qemu-devel@nongnu.org; Thu, 03 Mar 2016 06:17:52 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1abRGM-0005Xt-Vq for qemu-devel@nongnu.org; Thu, 03 Mar 2016 06:17:51 -0500 References: <1456998223-12356-1-git-send-email-arei.gonglei@huawei.com> <1456998223-12356-3-git-send-email-arei.gonglei@huawei.com> From: Paolo Bonzini Message-ID: <56D81D57.4040605@redhat.com> Date: Thu, 3 Mar 2016 12:17:43 +0100 MIME-Version: 1.0 In-Reply-To: <1456998223-12356-3-git-send-email-arei.gonglei@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/6] sheepdog: fix possible resouce leak and out-of-bounds access List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gonglei , qemu-devel@nongnu.org Cc: qemu-trivial@nongnu.org On 03/03/2016 10:43, Gonglei wrote: > CID 1352418 (#1 of 1): Out-of-bounds access (INCOMPATIBLE_CAST) > incompatible_cast: Pointer &snap_id points to an object whose effective > type is unsigned int (32 bits, unsigned) but is dereferenced as a wider > unsigned long (64 bits, unsigned). This may lead to memory corruption. > > We also need to free local_err when ret is not equals to 0. > > Signed-off-by: Gonglei > --- > block/sheepdog.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/block/sheepdog.c b/block/sheepdog.c > index 8739acc..3d81bba 100644 > --- a/block/sheepdog.c > +++ b/block/sheepdog.c > @@ -2543,7 +2543,7 @@ static int sd_snapshot_delete(BlockDriverState *bs, > const char *name, > Error **errp) > { > - uint32_t snap_id = 0; > + unsigned long snap_id = 0; > char snap_tag[SD_MAX_VDI_TAG_LEN]; > Error *local_err = NULL; > int fd, ret; > @@ -2565,20 +2565,23 @@ static int sd_snapshot_delete(BlockDriverState *bs, > memset(buf, 0, sizeof(buf)); > memset(snap_tag, 0, sizeof(snap_tag)); > pstrcpy(buf, SD_MAX_VDI_LEN, s->name); > - if (qemu_strtoul(snapshot_id, NULL, 10, (unsigned long *)&snap_id)) { > + if (qemu_strtoul(snapshot_id, NULL, 10, &snap_id)) { > return -1; > } > > if (snap_id) { > + assert(snap_id <= UINT_MAX); > + > hdr.snapid = snap_id; > } else { > pstrcpy(snap_tag, sizeof(snap_tag), snapshot_id); > pstrcpy(buf + SD_MAX_VDI_LEN, SD_MAX_VDI_TAG_LEN, snap_tag); > } > > - ret = find_vdi_name(s, s->name, snap_id, snap_tag, &vid, true, > + ret = find_vdi_name(s, s->name, hdr.snapid, snap_tag, &vid, true, > &local_err); > if (ret) { > + error_report_err(local_err); > return ret; > } > > A patch for this has been posted yesterday by Jeff Cody. Paolo