From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53674) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bz9Ps-0001xd-O8 for qemu-devel@nongnu.org; Tue, 25 Oct 2016 17:37:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bz9Pr-0004Qq-SG for qemu-devel@nongnu.org; Tue, 25 Oct 2016 17:37:52 -0400 Date: Tue, 25 Oct 2016 23:37:33 +0200 From: =?UTF-8?B?VG9tw6HFoSBHb2xlbWJpb3Zza8O9?= Message-ID: <20161025233733.4a0e2a67@fiorina> In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v5] raw_bsd: add offset and size options List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Kevin Wolf , Max Reitz , Markus Armbruster , Eric Blake , "Daniel P . Berrange" , qemu-block@nongnu.org I should test my code more before submitting it to ML. I have found two bugs in the patch. On Sun, 23 Oct 2016 16:54:37 +0200 Tom=C3=A1=C5=A1 Golembiovsk=C3=BD wrote: > +static int raw_read_options(QDict *options, BlockDriverState *bs, > + BDRVRawState *s, Error **errp) > +{ > + Error *local_err =3D NULL; > + QemuOpts *opts =3D NULL; > + int64_t real_size =3D 0; > + int ret; > + > + real_size =3D bdrv_getlength(bs->file->bs); > + if (real_size < 0) { > + error_setg_errno(errp, -real_size, "Could not get image size"); > + return real_size; > + } > + > + opts =3D qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort); > + qemu_opts_absorb_qdict(opts, options, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + ret =3D -EINVAL; > + goto end; > + } > + > + s->offset =3D qemu_opt_get_size(opts, "offset", 0); > + if (qemu_opt_find(opts, "size") !=3D NULL) { > + s->size =3D qemu_opt_get_size(opts, "size", 0); > + s->has_size =3D true; > + } else { > + s->has_size =3D false; > + s->size =3D real_size; This has to be: s->size =3D real_size - s->offset; .. to account for the offset. Otherwise the following check will fail. > + } > + > + /* Check size and offset */ > + if (real_size < s->offset || (real_size - s->offset) < s->size) { > + error_setg(errp, "The sum of offset (%" PRIu64 ") and size " > + "(%" PRIu64 ") has to be smaller or equal to the " > + " actual size of the containing file (%" PRId64 ")", > + s->offset, s->size, real_size); > + ret =3D -EINVAL; > + goto end; > + } > + > + /* Make sure size is multiple of BDRV_SECTOR_SIZE to prevent rounding > + * up and leaking out of the specified area. */ > + if (QEMU_IS_ALIGNED(s->size, BDRV_SECTOR_SIZE)) { The condition has to be negated. Silly mistake made while rewriting the condition to use QEMU_IS_ALIGNED. > + error_setg(errp, "Specified size is not multiple of %llu", > + BDRV_SECTOR_SIZE); > + ret =3D -EINVAL; > + goto end; > + } > + > + ret =3D 0; > + > +end: > + > + qemu_opts_del(opts); > + > + return ret; > +} > + --=20 Tom=C3=A1=C5=A1 Golembiovsk=C3=BD