From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [BUGFIX][PATCH 3/4] hvm_save_one: return correct data. Date: Sun, 22 Dec 2013 14:40:10 -0500 Message-ID: <52B7401A.5070809@terremark.com> References: <1386809777-12898-1-git-send-email-dslutz@terremark.com> <1386809777-12898-4-git-send-email-dslutz@terremark.com> <52AB25B4020000780010D0B0@nat28.tlf.novell.com> <52ACF7CE.9030904@terremark.com> <52ADDE15.8010408@citrix.com> <52AEC522020000780010D7BE@nat28.tlf.novell.com> <52AF3D91.6000809@terremark.com> <52AF4776.9000303@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------070905020301050201020301" Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vuot3-0004oY-OQ for xen-devel@lists.xenproject.org; Sun, 22 Dec 2013 19:40:29 +0000 In-Reply-To: <52AF4776.9000303@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Andrew Cooper Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , Ian Jackson , Don Slutz , Jan Beulich , xen-devel List-Id: xen-devel@lists.xenproject.org --------------070905020301050201020301 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit On 12/16/13 13:33, Andrew Cooper wrote: Not sure why it took till late 12/21 for me to get this e-mail. > On 16/12/2013 17:51, Don Slutz wrote: >> On 12/16/13 03:17, Jan Beulich wrote: >>>>>> On 15.12.13 at 17:51, Andrew Cooper wrote: >>>> On 15/12/2013 00:29, Don Slutz wrote: [snip] > Your loop condition needs to change be "off < (ctxt.cur - > sizeof(*desc))" otherwise the "off += sizeof(*desc)" can wander beyond > ctxt.cur in the loop body. You also need to verify that the > copy_to_guest doesn't exceed ctxt.cur. fixed. > Stylistically, "desc = (void *)ctxt.data + off;" needs to be "desc = > (void *)(ctxt.data + off);" as the latter is standards compliment C > while the former is UB which GCC has an extension to deal with sensibly. fixed. > Also you have a double space before sizeof in "off += sizeof(*desc);" Fixed. Version 4 attached. > ~Andrew > -Don Slutz --------------070905020301050201020301 Content-Type: text/x-patch; name="0001-hvm_save_one-return-correct-data.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-hvm_save_one-return-correct-data.patch" >>From 975028470091a9517111a409501e477ea50e02a6 Mon Sep 17 00:00:00 2001 From: Don Slutz Date: Tue, 12 Nov 2013 08:22:53 -0500 Subject: [BUGFIX][PATCH v4 1/1] hvm_save_one: return correct data. It is possible that hvm_sr_handlers[typecode].save does not use all the provided room. Also it can use variable sized records. In both cases, using: instance * hvm_sr_handlers[typecode].size does not select the correct instance. Add code to search for the correct instance. Signed-off-by: Don Slutz --- changes v3 to v4: adjust loop limit and copy_length. changes v2 to v3: merge in patch #4. changes v1 to v2: fix coding style and coding issues. xen/common/hvm/save.c | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index de76ada..2f8b687 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -98,9 +98,6 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, else sz = hvm_sr_handlers[typecode].size; - if ( (instance + 1) * hvm_sr_handlers[typecode].size > sz ) - return -EINVAL; - ctxt.size = sz; ctxt.data = xmalloc_bytes(sz); if ( !ctxt.data ) @@ -112,13 +109,30 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, d->domain_id, typecode); rv = -EFAULT; } - else if ( copy_to_guest(handle, - ctxt.data - + (instance * hvm_sr_handlers[typecode].size) - + sizeof (struct hvm_save_descriptor), - hvm_sr_handlers[typecode].size - - sizeof (struct hvm_save_descriptor)) ) - rv = -EFAULT; + else + { + uint32_t off; + struct hvm_save_descriptor *desc; + + rv = -EBADSLT; + for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += desc->length ) + { + desc = (void *)(ctxt.data + off); + /* Move past header */ + off += sizeof(*desc); + if ( instance == desc->instance ) + { + uint32_t copy_length = desc->length; + + if ( off + copy_length > ctxt.cur ) + copy_length = ctxt.cur - off; + rv = 0; + if ( copy_to_guest(handle, ctxt.data + off, copy_length) ) + rv = -EFAULT; + break; + } + } + } xfree(ctxt.data); return rv; -- 1.8.4 --------------070905020301050201020301 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --------------070905020301050201020301--