From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40194) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjneQ-0005GN-9v for qemu-devel@nongnu.org; Thu, 30 Oct 2014 07:12:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XjneK-00071T-4l for qemu-devel@nongnu.org; Thu, 30 Oct 2014 07:12:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44627) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XjneJ-00070U-U0 for qemu-devel@nongnu.org; Thu, 30 Oct 2014 07:12:16 -0400 Date: Thu, 30 Oct 2014 11:12:06 +0000 From: Stefan Hajnoczi Message-ID: <20141030111206.GE30746@stefanha-thinkpad.redhat.com> References: <1412772152-2427-1-git-send-email-gordongong0350@gmail.com> <20141028150402.GG22805@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="ytoMbUMiTKPMT3hY" Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH v5] Support vhd type VHD_DIFFERENCING List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Xiaodong Gong Cc: kwolf@redhat.com, Stefan Hajnoczi , qemu-devel@nongnu.org, ssdxiao@163.com --ytoMbUMiTKPMT3hY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Oct 29, 2014 at 09:30:31PM +0800, Xiaodong Gong wrote: > On 10/28/14, Stefan Hajnoczi wrote: > > On Wed, Oct 08, 2014 at 08:42:32PM +0800, Xiaodong Gong wrote: > >> +#define PLATFORM_MACX 0x5863614d /* big endian */ > >> +#define PLATFORM_W2RU 0x75723257 =2E.. > >> +static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header, > >> + BlockDriverState *bs, > >> + Error **errp) > >> +{ > >> + BDRVVPCState *s =3D bs->opaque; > >> + int64_t data_offset =3D 0; > >> + int data_length =3D 0; > >> + uint32_t platform; > >> + bool done =3D false; > >> + int parent_locator_offset =3D 0; > >> + int i; > >> + int ret =3D 0; > >> + > >> + for (i =3D 0; i < PARENT_LOCATOR_NUM; i++) { > >> + data_offset =3D > >> + be64_to_cpu(dyndisk_header->parent_locator[i].data_offset= ); > >> + data_length =3D > >> + be32_to_cpu(dyndisk_header->parent_locator[i].data_length= ); > >> + platform =3D dyndisk_header->parent_locator[i].platform; > > > > be32_to_cpu() missing? >=20 > this platform is big-ending QEMU compiles on both little-endian and big-endian hosts. You cannot define PLATFORM_* constants with the assumption that the host is little-endian because it won't work on big-endian hosts! > >> + > >> + /* Read location of backing file */ > >> + if (platform =3D=3D PLATFORM_MACX || platform =3D=3D PLATFORM= _W2RU) { > >> + if (data_offset > s->max_table_entries * s->block_size) { > >> + return -1; > >> + } > >> + if (data_length > BDRV_SECTOR_SIZE) { > >> + return -1; > >> + } > >> + ret =3D bdrv_pread(bs->file, data_offset, bs->backing_fil= e, > >> + data_length); > > > > Please check data_length against bs->backing_file[] size before reading > > into it. >=20 > upper data_length > BDRV_SECTOR_SIZE get this done I know but that assumes that BDRV_SECTOR_SIZE will always be less than sizeof(bs->backing_file[]) in the future. There must never be a buffer overflow, ever, even in the future when other parts of QEMU are changed. It's safer to check the size of bs->backing_file[] explicitly. > > > >> + if (ret < 0) { > >> + return ret; > >> + } > >> + bs->backing_file[data_length] =3D '\0'; > >> + } > >> + > >> + /* Convert location to ACSII string */ > >> + if (platform =3D=3D PLATFORM_MACX) { > >> + done =3D true; > >> + > >> + } else if (platform =3D=3D PLATFORM_W2RU) { > >> + /* Must be UTF16-LE to ASCII */ > > > > I guess this is where you wanted to use iconv? >=20 > I used the iconv first time, but changed it to the following things. > There are tow reasons, it could fail because the right codeset packet > is not installed and it must be UTF16-LE to ASCII. How about your ? I just wanted to make sure I understood the reason for #include correctly. How about using glib's charset conversion function? It seems a bit hacky to implement it manually (while ignoring the error cases if a UTF16-LE character doesn't map to ASCII!). --ytoMbUMiTKPMT3hY Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUUh0GAAoJEJykq7OBq3PIi5MIAKVhX8Hf46I9wlv3N9PS4uDq XZzALr1bVfb/jqBIST1Ybp5bkTnffIpAg/f/ecaFCtA8ksqLrglRzBvr7v80t2sK Zd7DEUOkCsw99m3rHuOn8HJZ3zf5q9AvnKLg9w2Hp9dLi5UX28THmhTqpXQStJam vp8VtiGzcTk+TA/ocMxK3IRxtT7Fjfd34ha87A0IiwssFMCTqz1QiZ9zfmkOorVs O50sCsB0Ekvrlr1lSRs/R3Dii5hhOC0JdQbOhFqjtT6SCPjn5sI4F7C+9bgcPIhA xtbdSS0lLTK6SPO/rCWxmWYX6+khXnJSHfhhLxJhptEb99EkAmtCngJaMmWZT3c= =vI5w -----END PGP SIGNATURE----- --ytoMbUMiTKPMT3hY--