diff mbox

[Hardy-xen,v2] SRU: Fix OMG how did this ever work (32bit)

Message ID 1308750171-12578-1-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader June 22, 2011, 1:42 p.m. UTC 
After Andy's feedback and looking a bit closer to what the 2.6.24
version of the file looks like, it seems that it is probably better
to refrain from changing too much code.

IOW it is better to look like 2.6.24 than like the 2.6.18 xen repo
we can check against.

This does actually reduce the changes to a few things that are much
simpler to look at. So I split the changes into three indipendant
findings. The first two are more theoretical but probably best fixed
right now when I remember.

The last one is the actual issue in pgd_dtor.

-Stefan

From e06452cbeb90be8fa2cb8752c6661b7443e5f834 Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Wed, 22 Jun 2011 14:34:18 +0200
Subject: [PATCH] UBUNTU: xen: Fix memory corruption caused by double free

The Xen patches missed to exclude a section in pgd_dtor which causes
pgds which are freed on a quicklist trim to be unlinked from the
pgd_list for a second time. This causes memory coruption in struct pages
which possible are not even related to pgds.

Beside of this, a possible NULL pointer access and potential unbalanced
function call to xen_destroy_region get fixed.

BugLink: http://bugs.launchpad.net/bugs/705562

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 ...n-Check-for-NULL-pointer-before-using-pgd.patch |   37 +++++++++++++++++
 ...xen_destroy_contiguous_region-only-needed.patch |   37 +++++++++++++++++
 ...Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch |   43 ++++++++++++++++++++
 3 files changed, 117 insertions(+), 0 deletions(-)
 create mode 100644 debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch
 create mode 100644 debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch
 create mode 100644 debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch

Comments

Tim Gardner June 22, 2011, 1:57 p.m. UTC | #1 
On 06/22/2011 07:42 AM, Stefan Bader wrote:
> After Andy's feedback and looking a bit closer to what the 2.6.24
> version of the file looks like, it seems that it is probably better
> to refrain from changing too much code.
>
> IOW it is better to look like 2.6.24 than like the 2.6.18 xen repo
> we can check against.
>
> This does actually reduce the changes to a few things that are much
> simpler to look at. So I split the changes into three indipendant
> findings. The first two are more theoretical but probably best fixed
> right now when I remember.
>
> The last one is the actual issue in pgd_dtor.
>
> -Stefan
>
>  From e06452cbeb90be8fa2cb8752c6661b7443e5f834 Mon Sep 17 00:00:00 2001
> From: Stefan Bader<stefan.bader@canonical.com>
> Date: Wed, 22 Jun 2011 14:34:18 +0200
> Subject: [PATCH] UBUNTU: xen: Fix memory corruption caused by double free
>
> The Xen patches missed to exclude a section in pgd_dtor which causes
> pgds which are freed on a quicklist trim to be unlinked from the
> pgd_list for a second time. This causes memory coruption in struct pages
> which possible are not even related to pgds.
>
> Beside of this, a possible NULL pointer access and potential unbalanced
> function call to xen_destroy_region get fixed.
>
> BugLink: http://bugs.launchpad.net/bugs/705562
>
> Signed-off-by: Stefan Bader<stefan.bader@canonical.com>
> ---
>   ...n-Check-for-NULL-pointer-before-using-pgd.patch |   37 +++++++++++++++++
>   ...xen_destroy_contiguous_region-only-needed.patch |   37 +++++++++++++++++
>   ...Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch |   43 ++++++++++++++++++++
>   3 files changed, 117 insertions(+), 0 deletions(-)
>   create mode 100644 debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch
>   create mode 100644 debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch
>   create mode 100644 debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch
>
> diff --git a/debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch b/debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch
> new file mode 100644
> index 0000000..5faf99a
> --- /dev/null
> +++ b/debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch
> @@ -0,0 +1,37 @@
> +From 310f9650b26f889008ae5210ed7cadb9dcb5e39f Mon Sep 17 00:00:00 2001
> +From: Stefan Bader<stefan.bader@canonical.com>
> +Date: Tue, 21 Jun 2011 09:48:56 +0200
> +Subject: [PATCH 21/23] UBUNTU: xen: Check for NULL pointer before using pgd
> +
> +In pgd_alloc(), the test for the new pgd being NULL is done
> +after it has already been used for test_and_unpin(). Do that
> +check before.
> +
> +BugLink: http://bugs.launchpad.net/bugs/705562
> +
> +Signed-off-by: Stefan Bader<stefan.bader@canonical.com>
> +---
> + arch/x86/mm/pgtable_32-xen.c |    5 ++++-
> + 1 files changed, 4 insertions(+), 1 deletions(-)
> +
> +diff --git a/arch/x86/mm/pgtable_32-xen.c b/arch/x86/mm/pgtable_32-xen.c
> +index 74a2e57..48242aa 100644
> +--- a/arch/x86/mm/pgtable_32-xen.c
> ++++ b/arch/x86/mm/pgtable_32-xen.c
> +@@ -345,9 +345,12 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
> + 	pmd_t **pmds = NULL;
> + 	unsigned long flags;
> +
> ++	if (!pgd)
> ++		return NULL;
> ++
> + 	pgd_test_and_unpin(pgd);
> +
> +-	if (PTRS_PER_PMD == 1 || !pgd)
> ++	if (PTRS_PER_PMD == 1)
> + 		return pgd;
> +
> + #ifdef CONFIG_XEN
> +--
> +1.7.4.1
> +
> diff --git a/debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch b/debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch
> new file mode 100644
> index 0000000..bb3670a
> --- /dev/null
> +++ b/debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch
> @@ -0,0 +1,37 @@
> +From 6a622c376c715a04d245bb3ca1f7910680b8165c Mon Sep 17 00:00:00 2001
> +From: Stefan Bader<stefan.bader@canonical.com>
> +Date: Wed, 22 Jun 2011 12:06:51 +0200
> +Subject: [PATCH 22/23] UBUNTU: xen: xen_destroy_contiguous_region only needed in unshared case
> +
> +The call to create is only done for !SHARED_KERNEL_PMD, so only destroy
> +in that case (also this is only useful when CONFIG_XEN is set.
> +
> +BugLink: http://bugs.launchpad.net/bugs/705562
> +
> +Signed-off-by: Stefan Bader<stefan.bader@canonical.com>
> +---
> + arch/x86/mm/pgtable_32-xen.c |    8 ++++++--
> + 1 files changed, 6 insertions(+), 2 deletions(-)
> +
> +diff --git a/arch/x86/mm/pgtable_32-xen.c b/arch/x86/mm/pgtable_32-xen.c
> +index 48242aa..49e28e7 100644
> +--- a/arch/x86/mm/pgtable_32-xen.c
> ++++ b/arch/x86/mm/pgtable_32-xen.c
> +@@ -473,8 +473,12 @@ void pgd_free(pgd_t *pgd)
> + 			pmd_cache_free(pmd, i);
> + 		}
> +
> +-		if (!xen_feature(XENFEAT_pae_pgdir_above_4gb))
> +-			xen_destroy_contiguous_region((unsigned long)pgd, 0);
> ++#ifdef CONFIG_XEN
> ++		if (!SHARED_KERNEL_PMD)
> ++			if (!xen_feature(XENFEAT_pae_pgdir_above_4gb))
> ++				xen_destroy_contiguous_region(
> ++					(unsigned long)pgd, 0);
> ++#endif
> + 	}
> +
> + 	/* in the non-PAE case, free_pgtables() clears user pgd entries */
> +--
> +1.7.4.1
> +
> diff --git a/debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch b/debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch
> new file mode 100644
> index 0000000..0c65d36
> --- /dev/null
> +++ b/debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch
> @@ -0,0 +1,43 @@
> +From b70dad384c6b97b30a11838989364bb5946922cf Mon Sep 17 00:00:00 2001
> +From: Stefan Bader<stefan.bader@canonical.com>
> +Date: Wed, 22 Jun 2011 12:16:10 +0200
> +Subject: [PATCH 23/23] UBUNTU: xen: Do not call pgd_list_del in pgd_dtor for Xen
> +
> +The Xen modifications add code to remove the pgd from the list as soon
> +as it is freed in pgd_free.
> +
> +BugLink: http://bugs.launchpad.net/bugs/705562
> +
> +Signed-off-by: Stefan Bader<stefan.bader@canonical.com>
> +---
> + arch/x86/mm/pgtable_32-xen.c |    5 +++++
> + 1 files changed, 5 insertions(+), 0 deletions(-)
> +
> +diff --git a/arch/x86/mm/pgtable_32-xen.c b/arch/x86/mm/pgtable_32-xen.c
> +index 49e28e7..d8bc211 100644
> +--- a/arch/x86/mm/pgtable_32-xen.c
> ++++ b/arch/x86/mm/pgtable_32-xen.c
> +@@ -290,15 +290,20 @@ static void pgd_ctor(void *pgd)
> +
> + static void pgd_dtor(void *pgd)
> + {
> ++#ifndef CONFIG_XEN
> + 	unsigned long flags; /* can be called from interrupt context */
> ++#endif
> +
> + 	if (SHARED_KERNEL_PMD)
> + 		return;
> +
> ++#ifndef CONFIG_XEN
> ++	/* This is done in pgd_free in the Xen case. */
> + 	paravirt_release_pd(__pa(pgd)>>  PAGE_SHIFT);
> + 	spin_lock_irqsave(&pgd_lock, flags);
> + 	pgd_list_del(pgd);
> + 	spin_unlock_irqrestore(&pgd_lock, flags);
> ++#endif
> +
> + 	pgd_test_and_unpin(pgd);
> + }
> +--
> +1.7.4.1
> +
diff mbox

Patch

diff --git a/debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch b/debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch
new file mode 100644
index 0000000..5faf99a
--- /dev/null
+++ b/debian/binary-custom.d/xen/patchset/022-UBUNTU-xen-Check-for-NULL-pointer-before-using-pgd.patch
@@ -0,0 +1,37 @@ 
+From 310f9650b26f889008ae5210ed7cadb9dcb5e39f Mon Sep 17 00:00:00 2001
+From: Stefan Bader <stefan.bader@canonical.com>
+Date: Tue, 21 Jun 2011 09:48:56 +0200
+Subject: [PATCH 21/23] UBUNTU: xen: Check for NULL pointer before using pgd
+
+In pgd_alloc(), the test for the new pgd being NULL is done
+after it has already been used for test_and_unpin(). Do that
+check before.
+
+BugLink: http://bugs.launchpad.net/bugs/705562
+
+Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
+---
+ arch/x86/mm/pgtable_32-xen.c |    5 ++++-
+ 1 files changed, 4 insertions(+), 1 deletions(-)
+
+diff --git a/arch/x86/mm/pgtable_32-xen.c b/arch/x86/mm/pgtable_32-xen.c
+index 74a2e57..48242aa 100644
+--- a/arch/x86/mm/pgtable_32-xen.c
++++ b/arch/x86/mm/pgtable_32-xen.c
+@@ -345,9 +345,12 @@ pgd_t *pgd_alloc(struct mm_struct *mm)
+ 	pmd_t **pmds = NULL;
+ 	unsigned long flags;
+ 
++	if (!pgd)
++		return NULL;
++
+ 	pgd_test_and_unpin(pgd);
+ 
+-	if (PTRS_PER_PMD == 1 || !pgd)
++	if (PTRS_PER_PMD == 1)
+ 		return pgd;
+ 
+ #ifdef CONFIG_XEN
+-- 
+1.7.4.1
+
diff --git a/debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch b/debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch
new file mode 100644
index 0000000..bb3670a
--- /dev/null
+++ b/debian/binary-custom.d/xen/patchset/023-UBUNTU-xen-xen_destroy_contiguous_region-only-needed.patch
@@ -0,0 +1,37 @@ 
+From 6a622c376c715a04d245bb3ca1f7910680b8165c Mon Sep 17 00:00:00 2001
+From: Stefan Bader <stefan.bader@canonical.com>
+Date: Wed, 22 Jun 2011 12:06:51 +0200
+Subject: [PATCH 22/23] UBUNTU: xen: xen_destroy_contiguous_region only needed in unshared case
+
+The call to create is only done for !SHARED_KERNEL_PMD, so only destroy
+in that case (also this is only useful when CONFIG_XEN is set.
+
+BugLink: http://bugs.launchpad.net/bugs/705562
+
+Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
+---
+ arch/x86/mm/pgtable_32-xen.c |    8 ++++++--
+ 1 files changed, 6 insertions(+), 2 deletions(-)
+
+diff --git a/arch/x86/mm/pgtable_32-xen.c b/arch/x86/mm/pgtable_32-xen.c
+index 48242aa..49e28e7 100644
+--- a/arch/x86/mm/pgtable_32-xen.c
++++ b/arch/x86/mm/pgtable_32-xen.c
+@@ -473,8 +473,12 @@ void pgd_free(pgd_t *pgd)
+ 			pmd_cache_free(pmd, i);
+ 		}
+ 
+-		if (!xen_feature(XENFEAT_pae_pgdir_above_4gb))
+-			xen_destroy_contiguous_region((unsigned long)pgd, 0);
++#ifdef CONFIG_XEN
++		if (!SHARED_KERNEL_PMD)
++			if (!xen_feature(XENFEAT_pae_pgdir_above_4gb))
++				xen_destroy_contiguous_region(
++					(unsigned long)pgd, 0);
++#endif
+ 	}
+ 
+ 	/* in the non-PAE case, free_pgtables() clears user pgd entries */
+-- 
+1.7.4.1
+
diff --git a/debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch b/debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch
new file mode 100644
index 0000000..0c65d36
--- /dev/null
+++ b/debian/binary-custom.d/xen/patchset/024-UBUNTU-xen-Do-not-call-pgd_list_del-in-pgd_dtor-for-.patch
@@ -0,0 +1,43 @@ 
+From b70dad384c6b97b30a11838989364bb5946922cf Mon Sep 17 00:00:00 2001
+From: Stefan Bader <stefan.bader@canonical.com>
+Date: Wed, 22 Jun 2011 12:16:10 +0200
+Subject: [PATCH 23/23] UBUNTU: xen: Do not call pgd_list_del in pgd_dtor for Xen
+
+The Xen modifications add code to remove the pgd from the list as soon
+as it is freed in pgd_free.
+
+BugLink: http://bugs.launchpad.net/bugs/705562
+
+Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
+---
+ arch/x86/mm/pgtable_32-xen.c |    5 +++++
+ 1 files changed, 5 insertions(+), 0 deletions(-)
+
+diff --git a/arch/x86/mm/pgtable_32-xen.c b/arch/x86/mm/pgtable_32-xen.c
+index 49e28e7..d8bc211 100644
+--- a/arch/x86/mm/pgtable_32-xen.c
++++ b/arch/x86/mm/pgtable_32-xen.c
+@@ -290,15 +290,20 @@ static void pgd_ctor(void *pgd)
+ 
+ static void pgd_dtor(void *pgd)
+ {
++#ifndef CONFIG_XEN
+ 	unsigned long flags; /* can be called from interrupt context */
++#endif
+ 
+ 	if (SHARED_KERNEL_PMD)
+ 		return;
+ 
++#ifndef CONFIG_XEN
++	/* This is done in pgd_free in the Xen case. */
+ 	paravirt_release_pd(__pa(pgd) >> PAGE_SHIFT);
+ 	spin_lock_irqsave(&pgd_lock, flags);
+ 	pgd_list_del(pgd);
+ 	spin_unlock_irqrestore(&pgd_lock, flags);
++#endif
+ 
+ 	pgd_test_and_unpin(pgd);
+ }
+-- 
+1.7.4.1
+