From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH] libxc: restore: bounds check for start_info.{store_mfn, console.domU.mfn} Date: Fri, 20 Jul 2012 13:00:32 -0400 Message-ID: <50098EB0.2000505@tycho.nsa.gov> References: <5e8449a87a993cc2d2fb.1342793618@cosworth.uk.xensource.com> <20489.33265.37325.676425@mariner.uk.xensource.com> <1342801825.26734.154.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1342801825.26734.154.camel@zakaz.uk.xensource.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: Ian Campbell Cc: Ian Jackson , Jonathan Ludlam , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org On 07/20/2012 12:30 PM, Ian Campbell wrote: > On Fri, 2012-07-20 at 17:06 +0100, Ian Jackson wrote: >> Ian Campbell writes ("[PATCH] libxc: restore: bounds check for start_info.{store_mfn, console.domU.mfn}"): >>> libxc: restore: bounds check for start_info.{store_mfn,console.domU.mfn} >>> >>> These fields are canonicalised by the guest on suspend and therefore must be >>> valid pfns during restore. >>> >>> Reported-by: Jonathan Ludlam >>> Signed-off-by: Ian Campbell >> >> Does this mean that a malicious restore file can take over the >> toolstack ? > > Good question, I should have considered this before posting. > > The value in question is used as an offset into the p2m. So this allows > the attacker to read off the end of that array, potentially reading some > other word and storing it in either *store_mfn or *console_mfn (or > both). Lets assume that the attacker is clever and can control some > value which can be seen in this way (perhaps the tools have a guest page > mapped which they control). > > The values are written to the attacker's guest's start info (harmful > only to themselves, I think) and used to seed a grant table entry. > Seeding the gnttab would allow the attacker to potentially grant access > to some other domain to one of the attacker's domain's own pages, which > again seems harmless enough. You cannot grant a page you do not own so > there is no way to leak information that way. > > The *foo_mfn pointers are arguments to the xc_domain_restore function > and are then used by the toolstack to write the mfns to xenstore and for > xs_domain_introduce (I can't see any other use in libxl/xl). > > I believe both xenconsoled and xenstored will default to using the grant > table entries seeded above these days, which will prevent them from > inadvertently mapping a page other than that owned by the attacher's > guest. Actually, it's just xenstored that was changed (oxenstored was not). I have a patch to do the same for xenconsoled saved for when 4.3 opens, but it was regarded as too late for 4.2 last time I mentioned it. > Some versions of those daemons use the mmap foreign privileged > interface. I suppose this could be used to trick xenconsoled into > treating an arbitrary page as the guests console or to trick xenstored > into treating an arbitrary page as a xenstore ring. I'm not sure if that > is dangerous or not. The map_foreign_range call does include a domain ID all the way up to the hypervisor, which prevents the daemons from mapping pages that the target domain in question isn't able to map on its own. -- Daniel De Graaf National Security Agency