From: Matt Wilson <msw@amazon.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Annie Li <annie.li@oracle.com>,
Steven Noonan <snoonan@amazon.com>, Matt Wilson <msw@amazon.com>,
xen-devel@lists.xen.org
Subject: [PATCH] xen/grant-table: correctly initialize grant table version 1
Date: Wed, 19 Dec 2012 20:20:14 +0000 [thread overview]
Message-ID: <1355948414-7503-1-git-send-email-msw@amazon.com> (raw)
Commit 85ff6acb075a484780b3d763fdf41596d8fc0970 (xen/granttable: Grant
tables V2 implementation) changed the GREFS_PER_GRANT_FRAME macro from
a constant to a conditional expression. The expression depends on
grant_table_version being appropriately set. Unfortunately, at init
time grant_table_version will be 0. The GREFS_PER_GRANT_FRAME
conditional expression checks for "grant_table_version == 1", and
therefore returns the number of grant references per frame for v2.
This causes gnttab_init() to allocate fewer pages for gnttab_list, as
a frame can old half the number of v2 entries than v1 entries. After
gnttab_resume() is called, grant_table_version is appropriately
set. nr_init_grefs will then be miscalculated and gnttab_free_count
will hold a value larger than the actual number of free gref entries.
If a guest is heavily utilizing improperly initialized v1 grant
tables, memory corruption can occur. One common manifestation is
corruption of the vmalloc list, resulting in a poisoned pointer
derefrence when accessing /proc/meminfo or /proc/vmallocinfo:
[ 40.770064] BUG: unable to handle kernel paging request at 0000200200001407
[ 40.770083] IP: [<ffffffff811a6fb0>] get_vmalloc_info+0x70/0x110
[ 40.770102] PGD 0
[ 40.770107] Oops: 0000 [#1] SMP
[ 40.770114] CPU 10
This patch introduces a static variable, grefs_per_grant_frame, to
cache the calculated value. gnttab_init() now calls
gnttab_request_version() early so that grant_table_version and
grefs_per_grant_frame can be appropriately set. A few BUG_ON()s have
been added to prevent this type of bug from reoccurring in the future.
Signed-off-by: Matt Wilson <msw@amazon.com>
Reviewed-and-Tested-by: Steven Noonan <snoonan@amazon.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Annie Li <annie.li@oracle.com>
Cc: xen-devel@lists.xen.org
---
drivers/xen/grant-table.c | 41 +++++++++++++++++++++++------------------
1 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/drivers/xen/grant-table.c b/drivers/xen/grant-table.c
index 043bf07..011fdc3 100644
--- a/drivers/xen/grant-table.c
+++ b/drivers/xen/grant-table.c
@@ -55,10 +55,6 @@
/* External tools reserve first few grant table entries. */
#define NR_RESERVED_ENTRIES 8
#define GNTTAB_LIST_END 0xffffffff
-#define GREFS_PER_GRANT_FRAME \
-(grant_table_version == 1 ? \
-(PAGE_SIZE / sizeof(struct grant_entry_v1)) : \
-(PAGE_SIZE / sizeof(union grant_entry_v2)))
static grant_ref_t **gnttab_list;
static unsigned int nr_grant_frames;
@@ -153,6 +149,7 @@ static struct gnttab_ops *gnttab_interface;
static grant_status_t *grstatus;
static int grant_table_version;
+static int grefs_per_grant_frame;
static struct gnttab_free_callback *gnttab_free_callback_list;
@@ -766,12 +763,14 @@ static int grow_gnttab_list(unsigned int more_frames)
unsigned int new_nr_grant_frames, extra_entries, i;
unsigned int nr_glist_frames, new_nr_glist_frames;
+ BUG_ON(grefs_per_grant_frame == 0);
+
new_nr_grant_frames = nr_grant_frames + more_frames;
- extra_entries = more_frames * GREFS_PER_GRANT_FRAME;
+ extra_entries = more_frames * grefs_per_grant_frame;
- nr_glist_frames = (nr_grant_frames * GREFS_PER_GRANT_FRAME + RPP - 1) / RPP;
+ nr_glist_frames = (nr_grant_frames * grefs_per_grant_frame + RPP - 1) / RPP;
new_nr_glist_frames =
- (new_nr_grant_frames * GREFS_PER_GRANT_FRAME + RPP - 1) / RPP;
+ (new_nr_grant_frames * grefs_per_grant_frame + RPP - 1) / RPP;
for (i = nr_glist_frames; i < new_nr_glist_frames; i++) {
gnttab_list[i] = (grant_ref_t *)__get_free_page(GFP_ATOMIC);
if (!gnttab_list[i])
@@ -779,12 +778,12 @@ static int grow_gnttab_list(unsigned int more_frames)
}
- for (i = GREFS_PER_GRANT_FRAME * nr_grant_frames;
- i < GREFS_PER_GRANT_FRAME * new_nr_grant_frames - 1; i++)
+ for (i = grefs_per_grant_frame * nr_grant_frames;
+ i < grefs_per_grant_frame * new_nr_grant_frames - 1; i++)
gnttab_entry(i) = i + 1;
gnttab_entry(i) = gnttab_free_head;
- gnttab_free_head = GREFS_PER_GRANT_FRAME * nr_grant_frames;
+ gnttab_free_head = grefs_per_grant_frame * nr_grant_frames;
gnttab_free_count += extra_entries;
nr_grant_frames = new_nr_grant_frames;
@@ -904,7 +903,8 @@ EXPORT_SYMBOL_GPL(gnttab_unmap_refs);
static unsigned nr_status_frames(unsigned nr_grant_frames)
{
- return (nr_grant_frames * GREFS_PER_GRANT_FRAME + SPP - 1) / SPP;
+ BUG_ON(grefs_per_grant_frame == 0);
+ return (nr_grant_frames * grefs_per_grant_frame + SPP - 1) / SPP;
}
static int gnttab_map_frames_v1(xen_pfn_t *frames, unsigned int nr_gframes)
@@ -1068,6 +1068,7 @@ static void gnttab_request_version(void)
rc = HYPERVISOR_grant_table_op(GNTTABOP_set_version, &gsv, 1);
if (rc == 0 && gsv.version == 2) {
grant_table_version = 2;
+ grefs_per_grant_frame = PAGE_SIZE / sizeof(union grant_entry_v2);
gnttab_interface = &gnttab_v2_ops;
} else if (grant_table_version == 2) {
/*
@@ -1080,10 +1081,9 @@ static void gnttab_request_version(void)
panic("we need grant tables version 2, but only version 1 is available");
} else {
grant_table_version = 1;
+ grefs_per_grant_frame = PAGE_SIZE / sizeof(struct grant_entry_v1);
gnttab_interface = &gnttab_v1_ops;
}
- printk(KERN_INFO "Grant tables using version %d layout.\n",
- grant_table_version);
}
int gnttab_resume(void)
@@ -1092,6 +1092,8 @@ int gnttab_resume(void)
char *kmsg = "Failed to kmalloc pages for pv in hvm grant frames\n";
gnttab_request_version();
+ printk(KERN_INFO "Grant tables using version %d layout.\n",
+ grant_table_version);
max_nr_gframes = gnttab_max_grant_frames();
if (max_nr_gframes < nr_grant_frames)
return -ENOSYS;
@@ -1137,9 +1139,10 @@ static int gnttab_expand(unsigned int req_entries)
int rc;
unsigned int cur, extra;
+ BUG_ON(grefs_per_grant_frame == 0);
cur = nr_grant_frames;
- extra = ((req_entries + (GREFS_PER_GRANT_FRAME-1)) /
- GREFS_PER_GRANT_FRAME);
+ extra = ((req_entries + (grefs_per_grant_frame-1)) /
+ grefs_per_grant_frame);
if (cur + extra > gnttab_max_grant_frames())
return -ENOSPC;
@@ -1157,21 +1160,23 @@ int gnttab_init(void)
unsigned int nr_init_grefs;
int ret;
+ gnttab_request_version();
nr_grant_frames = 1;
boot_max_nr_grant_frames = __max_nr_grant_frames();
/* Determine the maximum number of frames required for the
* grant reference free list on the current hypervisor.
*/
+ BUG_ON(grefs_per_grant_frame == 0);
max_nr_glist_frames = (boot_max_nr_grant_frames *
- GREFS_PER_GRANT_FRAME / RPP);
+ grefs_per_grant_frame / RPP);
gnttab_list = kmalloc(max_nr_glist_frames * sizeof(grant_ref_t *),
GFP_KERNEL);
if (gnttab_list == NULL)
return -ENOMEM;
- nr_glist_frames = (nr_grant_frames * GREFS_PER_GRANT_FRAME + RPP - 1) / RPP;
+ nr_glist_frames = (nr_grant_frames * grefs_per_grant_frame + RPP - 1) / RPP;
for (i = 0; i < nr_glist_frames; i++) {
gnttab_list[i] = (grant_ref_t *)__get_free_page(GFP_KERNEL);
if (gnttab_list[i] == NULL) {
@@ -1185,7 +1190,7 @@ int gnttab_init(void)
goto ini_nomem;
}
- nr_init_grefs = nr_grant_frames * GREFS_PER_GRANT_FRAME;
+ nr_init_grefs = nr_grant_frames * grefs_per_grant_frame;
for (i = NR_RESERVED_ENTRIES; i < nr_init_grefs - 1; i++)
gnttab_entry(i) = i + 1;
--
1.7.4.5
next reply other threads:[~2012-12-19 20:20 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-12-19 20:20 Matt Wilson [this message]
2012-12-20 3:42 ` [PATCH] xen/grant-table: correctly initialize grant table version 1 ANNIE LI
2012-12-20 21:24 ` Matt Wilson
2012-12-20 10:01 ` Ian Campbell
2012-12-20 21:19 ` Matt Wilson
[not found] <Message-ID: <1355997710.26722.12.camel@zakaz.uk.xensource.com>
2013-01-06 11:14 ` [PATCH v2] " Matt Wilson
[not found] ` <1357470882-17345-1-git-send-email-msw@amazon.com>
2013-01-09 2:40 ` ANNIE LI
[not found] ` <50ECD89A.50100@oracle.com>
2013-01-09 12:02 ` Ian Campbell
[not found] ` <1357732929.7989.250.camel@zakaz.uk.xensource.com>
2013-01-09 15:02 ` annie li
2013-01-10 9:16 ` Matt Wilson
[not found] ` <20130110091652.GB6316@u109add4315675089e695.ant.amazon.com>
2013-01-10 10:32 ` Ian Campbell
2013-01-10 11:02 ` ANNIE LI
2013-01-11 21:35 ` Konrad Rzeszutek Wilk
[not found] ` <20130111213550.GA31149@phenom.dumpdata.com>
2013-01-14 9:29 ` Matt Wilson
[not found] ` <20130114092936.GB15043@u109add4315675089e695.ant.amazon.com>
2013-01-14 15:31 ` Konrad Rzeszutek Wilk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1355948414-7503-1-git-send-email-msw@amazon.com \
--to=msw@amazon.com \
--cc=annie.li@oracle.com \
--cc=konrad.wilk@oracle.com \
--cc=snoonan@amazon.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).