From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>
Subject: Re: [PATCH 1/2] x86/HVM: simplify do_hvm_op()
Date: Tue, 25 Mar 2014 17:26:56 +0000 [thread overview]
Message-ID: <5331BC60.4040402@citrix.com> (raw)
In-Reply-To: <5331B6D70200007800001E19@nat28.tlf.novell.com>
[-- Attachment #1.1: Type: text/plain, Size: 4107 bytes --]
On 25/03/14 16:03, Jan Beulich wrote:
> - boundary checks in HVMOP_modified_memory, HVMOP_set_mem_type, and
> HVMOP_set_mem_access: all of these already check for overflow, so
> there's no need to range check the first _and_ last PFN (checking
> the last one suffices)
> - copying back interface structures that were previously copied from
> guest memory can use __copy_to_...(), since copy_from_...() already
> did the address range validation
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4288,7 +4288,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> a.value = d->arch.hvm_domain.params[a.index];
> break;
> }
> - rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> }
>
> HVM_DBG_LOG(DBG_LEVEL_HCALL, "%s param %u = %"PRIx64,
> @@ -4386,8 +4386,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> goto param_fail3;
>
> rc = -EINVAL;
> - if ( (a.first_pfn > domain_get_maximum_gpfn(d)) ||
> - ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> + if ( ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
> goto param_fail3;
>
> @@ -4416,7 +4415,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> /* Check for continuation if it's not the last interation */
> if ( a.nr > 0 && hypercall_preempt_check() )
> {
> - if ( copy_to_guest(arg, &a, 1) )
> + if ( __copy_to_guest(arg, &a, 1) )
> rc = -EFAULT;
> else
> rc = -EAGAIN;
> @@ -4465,7 +4464,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> a.mem_type = HVMMEM_ram_rw;
> else
> a.mem_type = HVMMEM_mmio_dm;
> - rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> }
>
> param_fail_getmemtype:
> @@ -4501,8 +4500,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> goto param_fail4;
>
> rc = -EINVAL;
> - if ( (a.first_pfn > domain_get_maximum_gpfn(d)) ||
> - ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> + if ( ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
> goto param_fail4;
>
> @@ -4558,7 +4556,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> /* Check for continuation if it's not the last interation */
> if ( a.nr > 0 && hypercall_preempt_check() )
> {
> - if ( copy_to_guest(arg, &a, 1) )
> + if ( __copy_to_guest(arg, &a, 1) )
> rc = -EFAULT;
> else
> rc = -EAGAIN;
> @@ -4595,9 +4593,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
>
> rc = -EINVAL;
> if ( (a.first_pfn != ~0ull) &&
> - ((a.first_pfn > domain_get_maximum_gpfn(d)) ||
> - ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> - ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d))) )
> + (((a.first_pfn + a.nr - 1) < a.first_pfn) ||
> + ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d))) )
> goto param_fail5;
>
> rc = p2m_set_mem_access(d, a.first_pfn, a.nr, a.hvmmem_access);
> @@ -4646,7 +4643,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
> goto param_fail6;
>
> a.hvmmem_access = access;
> - rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
> + rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
>
> param_fail6:
> rcu_unlock_domain(d);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 4965 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2014-03-25 17:27 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-25 15:51 [PATCH 0/2] x86/HVM: XSA-89 follow-ups Jan Beulich
2014-03-25 16:03 ` [PATCH 1/2] x86/HVM: simplify do_hvm_op() Jan Beulich
2014-03-25 17:26 ` Andrew Cooper [this message]
2014-03-25 16:03 ` [PATCH 2/2] x86/HVM: fix preemption handling in do_hvm_op() Jan Beulich
[not found] ` <CAGU+auuMgkBWobj=wpi6908mZuaYhHogc6j9F6gwdeUAGm=2DA@mail.gmail.com>
2014-03-25 18:40 ` Aravindh Puthiyaparambil (aravindp)
2014-03-26 10:06 ` Jan Beulich
2014-03-26 14:42 ` George Dunlap
2014-03-26 15:32 ` Ian Campbell
2014-03-26 15:44 ` Jan Beulich
2014-03-26 17:06 ` George Dunlap
2014-03-27 8:05 ` Jan Beulich
2014-03-27 10:16 ` Ian Campbell
2014-03-27 11:20 ` [PATCH 0/2] x86/HVM: XSA-89 follow-ups Tim Deegan
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=5331BC60.4040402@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=JBeulich@suse.com \
--cc=keir@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.