[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] x86/hvm: Fix use-after-free introduced by c/s 428607a
- To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxx>
- From: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
- Date: Tue, 2 Feb 2016 10:00:20 +0200
- Cc: Jan Beulich <JBeulich@xxxxxxxx>
- Comment: DomainKeys? See http://domainkeys.sourceforge.net/
- Delivery-date: Tue, 02 Feb 2016 08:00:26 +0000
- Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=bitdefender.com; b=jedG1Xhajm3gek9qjLZc6T0kkPfGSm3H2AVWNvixXtOd5QBeIR3T2RRigQLtEg0cdajQ17hpqEqDg99C74FyowBWhEhbhzN87t1hMp90gA+gp1s5M6KYYdbDentP5P+vZ7jJEvBrfBlH4bFG90Xd+iI/ua98jFYsC0utESj9r3O2IbgYRhWrKiTCQ3f4Cgc1NlQ3AqwCRIzqdSB6v3BmwZLc6fB9pV7UeLRrJewzHqX+abljxKPnHhJmA1vWSN8/jUj/K6Pc3cE9EFrDnz3btQn8P89EQoZGRkQsewvctlhbJea+QI1+x1chu2MWjYWw874BOws+JJg/vHijhuoMGw==; h=Received:Received:Received:Received:Received:Subject:To:References:Cc:From:Message-ID:Date:User-Agent:MIME-Version:In-Reply-To:Content-Type:Content-Transfer-Encoding:X-BitDefender-Scanner:X-BitDefender-Spam:X-BitDefender-SpamStamp:X-BitDefender-CF-Stamp;
- List-id: Xen developer discussion <xen-devel.lists.xen.org>
On 2/1/2016 7:56 PM, Andrew Cooper wrote:
c/s 428607a "x86: shrink 'struct domain', was already PAGE_SIZE" introduced a
use-after-free error during domain destruction, because of the order in which
timers are torn down.
(XEN) Xen call trace:
(XEN) [<ffff82d08013344e>] spinlock.c#check_lock+0x1e/0x40
(XEN) [<ffff82d08013349b>] _spin_lock+0x11/0x52
(XEN) [<ffff82d0801e8076>] vpt.c#pt_lock+0x24/0x40
(XEN) [<ffff82d0801e88f4>] destroy_periodic_time+0x18/0x81
(XEN) [<ffff82d0801e1089>] rtc_deinit+0x53/0x78
(XEN) [<ffff82d0801d1e5a>] hvm_domain_destroy+0x52/0x69
(XEN) [<ffff82d08016a758>] arch_domain_destroy+0x1a/0x98
(XEN) [<ffff82d080107cd5>] domain.c#complete_domain_destroy+0x6f/0x182
(XEN) [<ffff82d080126a19>] rcupdate.c#rcu_process_callbacks+0x144/0x1a6
(XEN) [<ffff82d080132c52>] softirq.c#__do_softirq+0x82/0x8d
(XEN) [<ffff82d080132caa>] do_softirq+0x13/0x15
(XEN) [<ffff82d080248ae1>] entry.o#process_softirqs+0x21/0x30
(XEN)
(XEN)
(XEN) ****************************************
(XEN) Panic on CPU 3:
(XEN) GENERAL PROTECTION FAULT
(XEN) [error_code=0000]
(XEN) ****************************************
Defer the freeing of d->arch.hvm_domain.pl_time until all timers have been
destroyed.
For safety, NULL out the pointers after freeing them, in an attempt to make
mistakes more obvious in the future.
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Corneliu ZUZU <czuzu@xxxxxxxxxxxxxxx>
---
xen/arch/x86/hvm/hvm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f24400d..38c65b3 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1674,8 +1674,10 @@ void hvm_domain_relinquish_resources(struct domain *d)
void hvm_domain_destroy(struct domain *d)
{
xfree(d->arch.hvm_domain.io_handler);
+ d->arch.hvm_domain.io_handler = NULL;
+
xfree(d->arch.hvm_domain.params);
- xfree(d->arch.hvm_domain.pl_time);
+ d->arch.hvm_domain.params = NULL;
hvm_destroy_cacheattr_region_list(d);
@@ -1686,6 +1688,9 @@ void hvm_domain_destroy(struct domain *d)
rtc_deinit(d);
stdvga_deinit(d);
vioapic_deinit(d);
+
+ xfree(d->arch.hvm_domain.pl_time);
+ d->arch.hvm_domain.pl_time = NULL;
}
static int hvm_save_tsc_adjust(struct domain *d, hvm_domain_context_t *h)
Ups, really sorry, ashamed to say I've done the mistake of not actually
testing this on a machine (working on ARM here). Won't happen again, if
I'm to send another patch I'll be sure to actually setup an X86 Dom0 &
at least do some basic tests.
Regarding the "set to NULL after free" practice, wouldn't it be wise to
define an xfreeandnull(void**) macro and use the "unsafe" xfree only
when it makes sense to (proly most of the time it won't)?
Corneliu.
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
