From patchwork Tue May 2 14:13:36 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Razvan Cojocaru X-Patchwork-Id: 9708139 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 447C46021C for ; Tue, 2 May 2017 14:15:43 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 342902845D for ; Tue, 2 May 2017 14:15:43 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 287242846A; Tue, 2 May 2017 14:15:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A83262845D for ; Tue, 2 May 2017 14:15:42 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d5YYZ-0004Av-Fj; Tue, 02 May 2017 14:13:35 +0000 Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d5YYY-0004Aa-2C for xen-devel@lists.xen.org; Tue, 02 May 2017 14:13:34 +0000 Received: from [85.158.137.68] by server-6.bemta-3.messagelabs.com id 67/30-02189-D0498095; Tue, 02 May 2017 14:13:33 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupmleJIrShJLcpLzFFi42KJPp6xUJdnCke kwecD2hZLPi5mcWD0OLr7N1MAYxRrZl5SfkUCa0bH4S7WgkaJimNNe1gbGJcLdTFycggJuEvs fb2VuYuRC8hewyjx7/AXVgjnGqPE9j1bgBwOsKq/84MhGhYySmw5pQxiCwsESzz5/JoNxBYRU Jbo/fWbBaSXWeAwo8T+D10sEINeMko8v/WLEaSKTcBQYvXGFrAOXgEniQ+b1zOB2CwCKhKL9v 9jBrFFBcIlmud/ZoSoEZQ4OfMJC4jNKWAv0bKxDayGWUBd4s+8S1C2uMStJ/OZIGx5ie1v54D FJQRyJLatfs8M8oCEgJTE/1YlkHskBGaySCxa+pIRokZG4tHEm2wTGMVmIVk3C8mKWUhWzEKy YgEjyypG9eLUorLUIl0LvaSizPSMktzEzBxdQwNjvdzU4uLE9NScxKRiveT83E2MwEhiAIIdj BfanQ8xSnIwKYnyqr9ijxTiS8pPqcxILM6ILyrNSS0+xCjDwaEkwSs1mSNSSLAoNT21Ii0zBx jTMGkJDh4lEd7Vk4DSvMUFibnFmekQqVOMilLivJ4gfQIgiYzSPLg2WBq5xCgrJczLCHSIEE9 BalFuZgmq/CtGcQ5GJWHeZpDxPJl5JXDTXwEtZgJa3CwLtrgkESEl1cDYvKJswfTdXecPyr6w 1ozRihM+JJthscFk2t8sNYbHGoVb/+12mZDes9ojcO45howr1zXv71HYmt38Sfq+2w7jBR4t7 e8XHnEQjt7yzc0u1ONzyiR1e9nIDVeW7j2hv9A/OEDVnOVurtlcz+9HZk5YuWDrhuSrr/9dS3 grdyfqq8ODooxknrpdSizFGYmGWsxFxYkArijH7B4DAAA= X-Env-Sender: rcojocaru@bitdefender.com X-Msg-Ref: server-16.tower-31.messagelabs.com!1493734412!90771983!1 X-Originating-IP: [91.199.104.161] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.4.12; banners=-,-,- X-VirusChecked: Checked Received: (qmail 3555 invoked from network); 2 May 2017 14:13:32 -0000 Received: from mx01.bbu.dsd.mx.bitdefender.com (HELO mx01.bbu.dsd.mx.bitdefender.com) (91.199.104.161) by server-16.tower-31.messagelabs.com with DHE-RSA-AES128-GCM-SHA256 encrypted SMTP; 2 May 2017 14:13:32 -0000 Received: (qmail 16257 invoked from network); 2 May 2017 17:13:31 +0300 Received: from unknown (HELO mx-sr.buh.bitdefender.com) (10.17.80.103) by mx01.bbu.dsd.mx.bitdefender.com with AES256-GCM-SHA384 encrypted SMTP; 2 May 2017 17:13:31 +0300 Received: from smtp03.buh.bitdefender.org (smtp.bitdefender.biz [10.17.80.77]) by mx-sr.buh.bitdefender.com (Postfix) with ESMTP id 3A0A77FBDE for ; Tue, 2 May 2017 17:13:31 +0300 (EEST) Received: (qmail 32639 invoked from network); 2 May 2017 17:13:31 +0300 Received: from rcojocaru.dsd.ro (HELO ?10.10.14.59?) (rcojocaru@bitdefender.com@10.10.14.59) by smtp03.buh.bitdefender.org with SMTP; 2 May 2017 17:13:30 +0300 To: Jan Beulich References: <1493731539-31798-1-git-send-email-rcojocaru@bitdefender.com> <5908AA410200007800155D60@prv-mh.provo.novell.com> <5908AF1F0200007800155DCA@prv-mh.provo.novell.com> From: Razvan Cojocaru Message-ID: <0e615fd3-bea9-99a5-fc11-ee0a887b6e80@bitdefender.com> Date: Tue, 2 May 2017 17:13:36 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 MIME-Version: 1.0 In-Reply-To: <5908AF1F0200007800155DCA@prv-mh.provo.novell.com> Content-Language: en-US X-BitDefender-Scanner: Clean, Agent: BitDefender qmail 3.1.6 on smtp03.buh.bitdefender.org, sigver: 7.71183 X-BitDefender-Spam: No (0) X-BitDefender-SpamStamp: Build: [Engines: 2.15.8.1074, Dats: 447919, Stamp: 3], Multi: [Enabled, t: (0.000010, 0.015782)], BW: [Enabled, t: (0.000021)], RBL DNSBL: [Disabled], APM: [Enabled, Score: 500, t: (0.005240), Flags: 85D2ED72; NN_LENGTH; NN_LEGIT_VALID_REPLY; NN_LEGIT_SUMM_400_WORDS; NN_NO_LINK_NMD; NN_LEGIT_BITDEFENDER; NN_LEGIT_S_SQARE_BRACKETS], SGN: [Enabled, t: (0.014152,0.000204)], URL: [Enabled, t: (0.000006)], RTDA: [Enabled, t: (0.328268), Hit: No, Details: v2.4.8; Id: 11.5ev3o7.1bf4fiev1.445j], total: 0(775) X-BitDefender-CF-Stamp: none Cc: tim@xen.org, sstabellini@kernel.org, wei.liu2@citrix.com, George.Dunlap@eu.citrix.com, andrew.cooper3@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org Subject: Re: [Xen-devel] [PATCH] xen/hvm: fix hypervisor crash with hvm_save_one() X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On 05/02/17 17:09, Jan Beulich wrote: >>>> On 02.05.17 at 15:54, wrote: >> On 05/02/17 16:48, Jan Beulich wrote: >>>>>> On 02.05.17 at 15:25, wrote: >>>> --- a/xen/common/hvm/save.c >>>> +++ b/xen/common/hvm/save.c >>>> @@ -113,7 +113,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, >>>> const struct hvm_save_descriptor *desc; >>>> >>>> rv = -ENOENT; >>>> - for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += desc->length >> ) >>>> + for ( off = 0; (off + sizeof(*desc)) < ctxt.cur; off += desc->length >> ) >>>> { >>>> desc = (void *)(ctxt.data + off); >>>> /* Move past header */ >>> >>> I don't think this is an appropriate fix. Instead I think the function >>> should check whether it got back any data at all, prior to entering >>> the loop. Furthermore it might be worth considering to (also) >>> refuse doing anything here if the domain's is_dying marker has >>> already been set. >> >> hvm_save_one() already checks is_dying: >> >> 77 /* Extract a single instance of a save record, by marshalling all >> 78 * records of that type and copying out the one we need. */ >> 79 int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t >> instance, >> 80 XEN_GUEST_HANDLE_64(uint8) handle) >> 81 { >> 82 int rv = 0; >> 83 size_t sz = 0; >> 84 struct vcpu *v; >> 85 hvm_domain_context_t ctxt = { 0, }; >> 86 >> 87 if ( d->is_dying >> 88 || typecode > HVM_SAVE_CODE_MAX >> 89 || hvm_sr_handlers[typecode].size < sizeof(struct >> hvm_save_descriptor) >> 90 || hvm_sr_handlers[typecode].save == NULL ) >> 91 return -EINVAL; > > Hmm, interesting. The timing window to see is_dying clear here, > bit no vCPU-s left there should be pretty small, so I wonder how > you've managed to hit it. But anyway ... > >> As for checking whether the handler wrote any data, I believe that >> Andrew has checked and none of the handlers report when no data is being >> passed on. > > ... that's not what I've read out of his replies. I don't think the > handlers need to report anything special. It is the caller which > should check whether, despite having got back "success" there's > no data in the buffer. So you would prefer something like this? { desc = (void *)(ctxt.data + off); @@ -132,6 +136,7 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, } } +out: xfree(ctxt.data); return rv; } Thanks, Razvan diff --git a/xen/common/hvm/save.c b/xen/common/hvm/save.c index 78706f5..d4c8d84 100644 --- a/xen/common/hvm/save.c +++ b/xen/common/hvm/save.c @@ -113,6 +113,10 @@ int hvm_save_one(struct domain *d, uint16_t typecode, uint16_t instance, const struct hvm_save_descriptor *desc; rv = -ENOENT; + + if ( !ctxt.cur ) + goto out; + for ( off = 0; off < (ctxt.cur - sizeof(*desc)); off += desc->length )