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: Sat, 14 Dec 2013 19:29:02 -0500 Message-ID: <52ACF7CE.9030904@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> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------050304070900030106000303" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Vrza8-0003nb-DR for xen-devel@lists.xenproject.org; Sun, 15 Dec 2013 00:29:16 +0000 In-Reply-To: <52AB25B4020000780010D0B0@nat28.tlf.novell.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: Jan Beulich Cc: Keir Fraser , Ian Campbell , Stefano Stabellini , Ian Jackson , Don Slutz , xen-devel List-Id: xen-devel@lists.xenproject.org --------------050304070900030106000303 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit On 12/13/13 09:20, Jan Beulich wrote: >>>> On 12.12.13 at 01:56, Don Slutz wrote: >> From: Don Slutz >> >> It is possible that hvm_sr_handlers[typecode].save does not use >> all the provided room. In that case, 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 > > Reviewed-by: Jan Beulich > > But this needs to be cleaned up coding style wise and ... > >> --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -112,13 >> +112,27 @@ 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; + + rv = -EBADSLT; + >> for (off = 0; off < ctxt.cur; off += >> hvm_sr_handlers[typecode].size) { + struct >> hvm_save_descriptor *desc + = (struct >> hvm_save_descriptor *)&ctxt.data[off]; > > .. this could be const, and the cast could simply be (void *), ... > >> + if (instance == desc->instance) { + rv >> = 0; + if ( copy_to_guest(handle, + >> ctxt.data + + off > > ... this doesn't need to be on a separate line, and ... > >> + + sizeof (struct >> hvm_save_descriptor), + >> hvm_sr_handlers[typecode].size + >> - sizeof (struct hvm_save_descriptor)) ) > > ... both these sizeof()s would now better be sizeof(*desc). > > Jan > I think I have corrected all coding errors (please check again). And done all requested changes. I did add the reviewed by (not sure if I should since this changes a large part of the patch, but they are all what Jan said). I have unit tested it and it appears to work the same as the previous version (as expected). Here is the new version, also attached. From e0e8f5246ba492b153884cea93bfe753f1b0782e Mon Sep 17 00:00:00 2001 From: Don Slutz Date: Tue, 12 Nov 2013 08:22:53 -0500 Subject: [PATCH v2 3/4] hvm_save_one: return correct data. It is possible that hvm_sr_handlers[typecode].save does not use all the provided room. In that case, 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 Reviewed-by: Jan Beulich --- xen/common/hvm/save.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index de76ada..6aaea6f 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -112,13 +112,27 @@ 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; + + rv = -EBADSLT; + for ( off = 0; off < ctxt.cur; off += hvm_sr_handlers[typecode].size ) + { + const struct hvm_save_descriptor *desc = (void *)&ctxt.data[off]; + + if ( instance == desc->instance ) + { + rv = 0; + if ( copy_to_guest(handle, + ctxt.data + off + sizeof(*desc), + hvm_sr_handlers[typecode].size + - sizeof(*desc)) ) + rv = -EFAULT; + break; + } + } + } xfree(ctxt.data); return rv; -- 1.7.11.7 -Don Slutz --------------050304070900030106000303 Content-Type: text/x-patch; name="0003-hvm_save_one-return-correct-data.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0003-hvm_save_one-return-correct-data.patch" >>From e0e8f5246ba492b153884cea93bfe753f1b0782e Mon Sep 17 00:00:00 2001 From: Don Slutz Date: Tue, 12 Nov 2013 08:22:53 -0500 Subject: [PATCH v2 3/4] hvm_save_one: return correct data. It is possible that hvm_sr_handlers[typecode].save does not use all the provided room. In that case, 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 Reviewed-by: Jan Beulich --- xen/common/hvm/save.c | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index de76ada..6aaea6f 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -112,13 +112,27 @@ 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; + + rv = -EBADSLT; + for ( off = 0; off < ctxt.cur; off += hvm_sr_handlers[typecode].size ) + { + const struct hvm_save_descriptor *desc = (void *)&ctxt.data[off]; + + if ( instance == desc->instance ) + { + rv = 0; + if ( copy_to_guest(handle, + ctxt.data + off + sizeof(*desc), + hvm_sr_handlers[typecode].size + - sizeof(*desc)) ) + rv = -EFAULT; + break; + } + } + } xfree(ctxt.data); return rv; -- 1.7.11.7 --------------050304070900030106000303 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 --------------050304070900030106000303--