From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] tools/libxc: Don't leave scratch_pfn uninitialised if the domain has no memory Date: Fri, 30 Jan 2015 00:17:07 +0000 Message-ID: <54CACD83.80700@citrix.com> References: <1422460355-16163-1-git-send-email-andrew.cooper3@citrix.com> <54CA7D88.5050702@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <54CA7D88.5050702@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall , Xen-devel Cc: Wei Liu , Ian Jackson , Ian Campbell , Jan Beulich List-Id: xen-devel@lists.xenproject.org On 29/01/2015 18:35, Julien Grall wrote: > Hi Andrew, > > On 28/01/15 15:52, Andrew Cooper wrote: >> c/s 5b5c40c0d1 "libxc: introduce a per architecture scratch pfn for temporary >> grant mapping" accidentally an issue whereby there were two paths out of >> xc_core_arch_get_scratch_gpfn() which returned 0, but only one of which >> assigned a value to the gpfn parameter. >> xc_domain_maximum_gpfn() can validly return 0, at which point gpfn 1 is a >> valid scratch page to use. > The original version was considering rc = 0 as an error. Should not we > keep the same behavior? > > Regards, The difference between this code and the original is that the original returned two bits of information in its return value, whereas this has return value and a parameter it fills in. Independent of whether 0 should be a success or failure, the existing caller used 0 as a success case and used an uninitialised piece of stack as a scratch pfn. As stated in the commit message, I believe that if 0 is the max memory so far, 1 is a valid scratch pfn to use, making 0 from the hypercall a valid success case. ~Andrew