From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
"Jan Beulich" <JBeulich@suse.com>,
"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
"Stefano Stabellini" <sstabellini@kernel.org>,
"Julien Grall" <julien@xen.org>,
"Volodymyr Babchuk" <Volodymyr_Babchuk@epam.com>,
"Paul Durrant" <paul@xen.org>,
"Oleksandr Tyshchenko" <oleksandr_tyshchenko@epam.com>
Subject: [PATCH v8 01/16] xen/memory: Reject out-of-range resource 'frame' values
Date: Sat, 30 Jan 2021 02:58:37 +0000 [thread overview]
Message-ID: <20210130025852.12430-2-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20210130025852.12430-1-andrew.cooper3@citrix.com>
The ABI is unfortunate, and frame being 64 bits leads to all kinds of problems
performing correct overflow checks.
Reject out-of-range values, and combinations which overflow, and use unsigned
int consistently elsewhere. This fixes several truncation bugs in the grant
call tree, as the underlying limits are expressed with unsigned int to begin
with.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien@xen.org>
CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
CC: Paul Durrant <paul@xen.org>
CC: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
v8:
* Push long=>int conversion all the way down into gnttab/ioreq
* Rebase over ARM/IOREQ series
v7.5:
* New
---
xen/common/grant_table.c | 14 +++++++-------
xen/common/ioreq.c | 2 +-
xen/common/memory.c | 17 +++++++++++++++--
xen/include/xen/grant_table.h | 8 ++++----
xen/include/xen/ioreq.h | 2 +-
5 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index f6f5acd300..4ecec35f28 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -3918,7 +3918,7 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
/* caller must hold write lock */
static int gnttab_get_status_frame_mfn(struct domain *d,
- unsigned long idx, mfn_t *mfn)
+ unsigned int idx, mfn_t *mfn)
{
const struct grant_table *gt = d->grant_table;
@@ -3929,8 +3929,8 @@ static int gnttab_get_status_frame_mfn(struct domain *d,
if ( idx >= nr_status_frames(gt) )
{
- unsigned long nr_status;
- unsigned long nr_grant;
+ unsigned int nr_status;
+ unsigned int nr_grant;
nr_status = idx + 1; /* sufficient frames to make idx valid */
@@ -3958,7 +3958,7 @@ static int gnttab_get_status_frame_mfn(struct domain *d,
/* caller must hold write lock */
static int gnttab_get_shared_frame_mfn(struct domain *d,
- unsigned long idx, mfn_t *mfn)
+ unsigned int idx, mfn_t *mfn)
{
const struct grant_table *gt = d->grant_table;
@@ -3966,7 +3966,7 @@ static int gnttab_get_shared_frame_mfn(struct domain *d,
if ( idx >= nr_grant_frames(gt) )
{
- unsigned long nr_grant;
+ unsigned int nr_grant;
nr_grant = idx + 1; /* sufficient frames to make idx valid */
@@ -4021,7 +4021,7 @@ int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn, mfn_t *mfn)
return rc;
}
-int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
+int gnttab_get_shared_frame(struct domain *d, unsigned int idx,
mfn_t *mfn)
{
struct grant_table *gt = d->grant_table;
@@ -4034,7 +4034,7 @@ int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
return rc;
}
-int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+int gnttab_get_status_frame(struct domain *d, unsigned int idx,
mfn_t *mfn)
{
struct grant_table *gt = d->grant_table;
diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index a36137d41d..90ed2e0302 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -771,7 +771,7 @@ static int ioreq_server_get_info(struct domain *d, ioservid_t id,
}
int ioreq_server_get_frame(struct domain *d, ioservid_t id,
- unsigned long idx, mfn_t *mfn)
+ unsigned int idx, mfn_t *mfn)
{
struct ioreq_server *s;
int rc;
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 33296e65cb..4d681597a5 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -1055,7 +1055,7 @@ static long xatp_permission_check(struct domain *d, unsigned int space)
}
static int acquire_grant_table(struct domain *d, unsigned int id,
- unsigned long frame,
+ unsigned int frame,
unsigned int nr_frames,
xen_pfn_t mfn_list[])
{
@@ -1094,7 +1094,7 @@ static int acquire_grant_table(struct domain *d, unsigned int id,
static int acquire_ioreq_server(struct domain *d,
unsigned int id,
- unsigned long frame,
+ unsigned int frame,
unsigned int nr_frames,
xen_pfn_t mfn_list[])
{
@@ -1164,6 +1164,19 @@ static int acquire_resource(
if ( xmar.nr_frames > ARRAY_SIZE(mfn_list) )
return -E2BIG;
+ /*
+ * The ABI is rather unfortunate. nr_frames (and therefore the total size
+ * of the resource) is 32bit, while frame (the offset within the resource
+ * we'd like to start at) is 64bit.
+ *
+ * Reject values oustide the of the range of nr_frames, as well as
+ * combinations of frame and nr_frame which overflow, to simplify the rest
+ * of the logic.
+ */
+ if ( (xmar.frame >> 32) ||
+ ((xmar.frame + xmar.nr_frames) >> 32) )
+ return -EINVAL;
+
rc = rcu_lock_remote_domain_by_id(xmar.domid, &d);
if ( rc )
return rc;
diff --git a/xen/include/xen/grant_table.h b/xen/include/xen/grant_table.h
index 8876f1f28e..6d14fe2526 100644
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -55,9 +55,9 @@ int mem_sharing_gref_to_gfn(struct grant_table *gt, grant_ref_t ref,
int gnttab_map_frame(struct domain *d, unsigned long idx, gfn_t gfn,
mfn_t *mfn);
-int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
+int gnttab_get_shared_frame(struct domain *d, unsigned int idx,
mfn_t *mfn);
-int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+int gnttab_get_status_frame(struct domain *d, unsigned int idx,
mfn_t *mfn);
#else
@@ -92,13 +92,13 @@ static inline int gnttab_map_frame(struct domain *d, unsigned long idx,
return -EINVAL;
}
-static inline int gnttab_get_shared_frame(struct domain *d, unsigned long idx,
+static inline int gnttab_get_shared_frame(struct domain *d, unsigned int idx,
mfn_t *mfn)
{
return -EINVAL;
}
-static inline int gnttab_get_status_frame(struct domain *d, unsigned long idx,
+static inline int gnttab_get_status_frame(struct domain *d, unsigned int idx,
mfn_t *mfn)
{
return -EINVAL;
diff --git a/xen/include/xen/ioreq.h b/xen/include/xen/ioreq.h
index 2d635e9432..a54a637bef 100644
--- a/xen/include/xen/ioreq.h
+++ b/xen/include/xen/ioreq.h
@@ -90,7 +90,7 @@ bool vcpu_ioreq_handle_completion(struct vcpu *v);
bool is_ioreq_server_page(struct domain *d, const struct page_info *page);
int ioreq_server_get_frame(struct domain *d, ioservid_t id,
- unsigned long idx, mfn_t *mfn);
+ unsigned int idx, mfn_t *mfn);
int ioreq_server_map_mem_type(struct domain *d, ioservid_t id,
uint32_t type, uint32_t flags);
--
2.11.0
next prev parent reply other threads:[~2021-01-30 2:59 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-30 2:58 [PATCH v8 00/16] acquire_resource size and external IPT monitoring Andrew Cooper
2021-01-30 2:58 ` Andrew Cooper [this message]
2021-01-30 2:58 ` [PATCH v8 02/16] xen/gnttab: Rework resource acquisition Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 03/16] xen/memory: Fix acquire_resource size semantics Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 04/16] xen/memory: Improve compat XENMEM_acquire_resource handling Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 05/16] xen/memory: Indent part of acquire_resource() Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 06/16] xen/memory: Fix mapping grant tables with XENMEM_acquire_resource Andrew Cooper
2021-02-01 10:10 ` Roger Pau Monné
2021-02-01 11:11 ` Andrew Cooper
2021-02-01 12:07 ` Roger Pau Monné
2021-02-01 12:10 ` Andrew Cooper
2021-02-01 13:03 ` Jan Beulich
2021-02-01 14:04 ` Andrew Cooper
2021-02-01 14:32 ` Jan Beulich
2021-01-30 2:58 ` [PATCH v8 07/16] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vmtrace Andrew Cooper
2021-02-01 10:32 ` Roger Pau Monné
2021-01-30 2:58 ` [PATCH v8 08/16] xen/domain: Add vmtrace_size domain creation parameter Andrew Cooper
2021-02-01 10:51 ` Roger Pau Monné
2021-02-01 11:19 ` Andrew Cooper
2021-02-01 13:18 ` Jan Beulich
2021-02-01 14:22 ` Andrew Cooper
2021-02-01 14:36 ` Jan Beulich
2021-02-01 22:14 ` Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 09/16] tools/[lib]xl: Add vmtrace_buf_size parameter Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 10/16] xen/memory: Add a vmtrace_buf resource type Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 11/16] x86/vmx: Add Intel Processor Trace support Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 12/16] xen/domctl: Add XEN_DOMCTL_vmtrace_op Andrew Cooper
2021-02-01 12:01 ` Roger Pau Monné
2021-02-01 13:00 ` Andrew Cooper
2021-02-01 14:27 ` Roger Pau Monné
2021-01-30 2:58 ` [PATCH v8 13/16] tools/libxc: Add xc_vmtrace_* functions Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 14/16] tools/misc: Add xen-vmtrace tool Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 15/16] xen/vmtrace: support for VM forks Andrew Cooper
2021-01-30 2:58 ` [PATCH v8 16/16] x86/vm_event: Carry the vmtrace buffer position in vm_event Andrew Cooper
2021-02-01 9:51 ` Jan Beulich
2021-02-01 12:34 ` [PATCH v8 00/16] acquire_resource size and external IPT monitoring Oleksandr
2021-02-01 13:07 ` Andrew Cooper
2021-02-01 13:47 ` Oleksandr
2021-02-01 14:00 ` Andrew Cooper
2021-02-02 12:09 ` Oleksandr
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=20210130025852.12430-2-andrew.cooper3@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=julien@xen.org \
--cc=oleksandr_tyshchenko@epam.com \
--cc=paul@xen.org \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.