From: Jan Beulich <jbeulich@suse.com>
To: Alexandru Stefan ISAILA <aisaila@bitdefender.com>
Cc: "Petre Ovidiu PIRCALABU" <ppircalabu@bitdefender.com>,
"Kevin Tian" <kevin.tian@intel.com>,
"Tamas K Lengyel" <tamas@tklengyel.com>, "Wei Liu" <wl@xen.org>,
"Razvan COJOCARU" <rcojocaru@bitdefender.com>,
"George Dunlap" <george.dunlap@eu.citrix.com>,
"Andrew Cooper" <andrew.cooper3@citrix.com>,
"Jun Nakajima" <jun.nakajima@intel.com>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH V4 1/4] x86/mm: Add array_index_nospec to guest provided index values
Date: Tue, 17 Dec 2019 17:50:15 +0100 [thread overview]
Message-ID: <9012ab5f-4a3b-d469-6c03-20aa59773c2a@suse.com> (raw)
In-Reply-To: <20191217151144.9781-1-aisaila@bitdefender.com>
On 17.12.2019 16:12, Alexandru Stefan ISAILA wrote:
> --- a/xen/arch/x86/mm/mem_access.c
> +++ b/xen/arch/x86/mm/mem_access.c
> @@ -367,10 +367,11 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn, uint32_t nr,
> if ( altp2m_idx )
> {
> if ( altp2m_idx >= MAX_ALTP2M ||
> - d->arch.altp2m_eptp[altp2m_idx] == mfn_x(INVALID_MFN) )
> + d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
The bounds check is against MAX_ALTP2M. Both MAX_ values look to be
independent, which means bounds check and value passed to the
helper need to match up (not just here).
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1353,7 +1353,8 @@ void setup_ept_dump(void)
>
> void p2m_init_altp2m_ept(struct domain *d, unsigned int i)
> {
> - struct p2m_domain *p2m = d->arch.altp2m_p2m[i];
> + struct p2m_domain *p2m =
> + d->arch.altp2m_p2m[array_index_nospec(i, MAX_ALTP2M)];
> struct p2m_domain *hostp2m = p2m_get_hostp2m(d);
> struct ept_data *ept;
>
> @@ -1366,7 +1367,7 @@ void p2m_init_altp2m_ept(struct domain *d, unsigned int i)
> p2m->max_mapped_pfn = p2m->max_remapped_gfn = 0;
> ept = &p2m->ept;
> ept->mfn = pagetable_get_pfn(p2m_get_pagetable(p2m));
> - d->arch.altp2m_eptp[i] = ept->eptp;
> + d->arch.altp2m_eptp[array_index_nospec(i, MAX_EPTP)] = ept->eptp;
> }
>
> unsigned int p2m_find_altp2m_by_eptp(struct domain *d, uint64_t eptp)
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -2499,7 +2499,7 @@ static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
> struct p2m_domain *p2m;
>
> ASSERT(idx < MAX_ALTP2M);
> - p2m = d->arch.altp2m_p2m[idx];
> + p2m = d->arch.altp2m_p2m[array_index_nospec(idx, MAX_ALTP2M)];
>
> p2m_lock(p2m);
>
> @@ -2540,7 +2540,7 @@ static int p2m_activate_altp2m(struct domain *d, unsigned int idx)
>
> ASSERT(idx < MAX_ALTP2M);
>
> - p2m = d->arch.altp2m_p2m[idx];
> + p2m = d->arch.altp2m_p2m[array_index_nospec(idx, MAX_ALTP2M)];
All of the above have a more or less significant disconnect between
the bounds check and the use as array index. I think it would be
quite helpful if these could live close to one another, so one can
(see further up) easily prove that both specified bounds actually
match up.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-12-17 16:50 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-17 15:12 [Xen-devel] [PATCH V4 1/4] x86/mm: Add array_index_nospec to guest provided index values Alexandru Stefan ISAILA
2019-12-17 15:12 ` [Xen-devel] [PATCH V4 2/4] x86/altp2m: Add hypercall to set a range of sve bits Alexandru Stefan ISAILA
2019-12-17 17:00 ` Jan Beulich
2019-12-18 8:13 ` Alexandru Stefan ISAILA
2019-12-18 8:45 ` Alexandru Stefan ISAILA
2019-12-18 10:18 ` Jan Beulich
2019-12-18 10:32 ` Alexandru Stefan ISAILA
2019-12-17 15:12 ` [Xen-devel] [PATCH V4 3/4] x86/mm: Pull out the p2m specifics from p2m_init_altp2m_ept Alexandru Stefan ISAILA
2019-12-18 10:36 ` Jan Beulich
2019-12-17 15:12 ` [Xen-devel] [PATCH V4 4/4] x86/mm: Make use of the default access param from xc_altp2m_create_view Alexandru Stefan ISAILA
2019-12-18 10:45 ` Jan Beulich
2019-12-18 11:53 ` Alexandru Stefan ISAILA
2019-12-17 15:21 ` [Xen-devel] [PATCH V4 1/4] x86/mm: Add array_index_nospec to guest provided index values Tamas K Lengyel
2019-12-17 16:50 ` Jan Beulich [this message]
2019-12-18 8:06 ` Alexandru Stefan ISAILA
2019-12-18 9:57 ` Alexandru Stefan ISAILA
2019-12-18 10:06 ` Jan Beulich
2019-12-18 10:26 ` Alexandru Stefan ISAILA
2019-12-18 9:59 ` Jan Beulich
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=9012ab5f-4a3b-d469-6c03-20aa59773c2a@suse.com \
--to=jbeulich@suse.com \
--cc=aisaila@bitdefender.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@eu.citrix.com \
--cc=jun.nakajima@intel.com \
--cc=kevin.tian@intel.com \
--cc=ppircalabu@bitdefender.com \
--cc=rcojocaru@bitdefender.com \
--cc=roger.pau@citrix.com \
--cc=tamas@tklengyel.com \
--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.