From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] vNUMA: validate XEN_DOMCTL_setvnumainfo input Date: Tue, 3 Mar 2015 20:17:38 +0000 Message-ID: <54F616E2.3090800@citrix.com> References: <54F5D56B0200007800065BB9@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6335061490371407491==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YStGC-0002MS-Up for xen-devel@lists.xenproject.org; Tue, 03 Mar 2015 20:17:45 +0000 In-Reply-To: <54F5D56B0200007800065BB9@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Keir Fraser , Ian Jackson , Dario Faggioli , Tim Deegan , Ian Campbell , Wei Liu List-Id: xen-devel@lists.xenproject.org --===============6335061490371407491== Content-Type: multipart/alternative; boundary="------------030508040400070006090404" --------------030508040400070006090404 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit On 03/03/15 14:38, Jan Beulich wrote: > As we get ready to use the information set for a domain here we should > make sure it is actually valid: Both vNode and pNode numbers should be > in range. Do a little bit of other cleanup so the code ends up looking > reasonably consistent in style. > > Along with this goes that we don't need an array of unsigned int to > store the pNode number - a nodeid_t one (a quarter the size) suffices. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper > > --- a/xen/common/domctl.c > +++ b/xen/common/domctl.c > @@ -344,7 +344,7 @@ static struct vnuma_info *vnuma_alloc(un > > vnuma->vdistance = xmalloc_array(unsigned int, nr_vnodes * nr_vnodes); > vnuma->vcpu_to_vnode = xmalloc_array(unsigned int, nr_vcpus); > - vnuma->vnode_to_pnode = xmalloc_array(unsigned int, nr_vnodes); > + vnuma->vnode_to_pnode = xmalloc_array(nodeid_t, nr_vnodes); > vnuma->vmemrange = xmalloc_array(xen_vmemrange_t, nr_ranges); > > if ( vnuma->vdistance == NULL || vnuma->vmemrange == NULL || > @@ -382,30 +382,40 @@ static struct vnuma_info *vnuma_init(con > nr_vnodes * nr_vnodes) ) > goto vnuma_fail; > > + if ( copy_from_guest(info->vmemrange, uinfo->vmemrange, > + uinfo->nr_vmemranges) ) > + goto vnuma_fail; > + > if ( copy_from_guest(info->vcpu_to_vnode, uinfo->vcpu_to_vnode, > d->max_vcpus) ) > goto vnuma_fail; > > - if ( copy_from_guest(info->vnode_to_pnode, uinfo->vnode_to_pnode, > - nr_vnodes) ) > - goto vnuma_fail; > + ret = -E2BIG; > + for ( i = 0; i < d->max_vcpus; ++i ) > + if ( info->vcpu_to_vnode[i] >= nr_vnodes ) > + goto vnuma_fail; > > - if (copy_from_guest(info->vmemrange, uinfo->vmemrange, > - uinfo->nr_vmemranges)) > - goto vnuma_fail; > + for ( i = 0; i < nr_vnodes; ++i ) > + { > + unsigned int pnode; > + > + ret = -EFAULT; > + if ( copy_from_guest_offset(&pnode, uinfo->vnode_to_pnode, i, 1) ) > + goto vnuma_fail; > + ret = -E2BIG; > + if ( pnode >= MAX_NUMNODES ) > + goto vnuma_fail; > + info->vnode_to_pnode[i] = pnode; > + } > > info->nr_vnodes = nr_vnodes; > info->nr_vmemranges = uinfo->nr_vmemranges; > > /* Check that vmemranges flags are zero. */ > + ret = -EINVAL; > for ( i = 0; i < info->nr_vmemranges; i++ ) > - { > if ( info->vmemrange[i].flags != 0 ) > - { > - ret = -EINVAL; > goto vnuma_fail; > - } > - } > > return info; > > --- a/xen/include/xen/domain.h > +++ b/xen/include/xen/domain.h > @@ -4,6 +4,7 @@ > > #include > #include > +#include > > typedef union { > struct vcpu_guest_context *nat; > @@ -99,7 +100,7 @@ struct vnuma_info { > unsigned int nr_vmemranges; > unsigned int *vdistance; > unsigned int *vcpu_to_vnode; > - unsigned int *vnode_to_pnode; > + nodeid_t *vnode_to_pnode; > struct xen_vmemrange *vmemrange; > }; > > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------030508040400070006090404 Content-Type: text/html; charset="windows-1252" Content-Transfer-Encoding: 7bit
On 03/03/15 14:38, Jan Beulich wrote:
As we get ready to use the information set for a domain here we should
make sure it is actually valid: Both vNode and pNode numbers should be
in range. Do a little bit of other cleanup so the code ends up looking
reasonably consistent in style.

Along with this goes that we don't need an array of unsigned int to
store the pNode number - a nodeid_t one (a quarter the size) suffices.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -344,7 +344,7 @@ static struct vnuma_info *vnuma_alloc(un
 
     vnuma->vdistance = xmalloc_array(unsigned int, nr_vnodes * nr_vnodes);
     vnuma->vcpu_to_vnode = xmalloc_array(unsigned int, nr_vcpus);
-    vnuma->vnode_to_pnode = xmalloc_array(unsigned int, nr_vnodes);
+    vnuma->vnode_to_pnode = xmalloc_array(nodeid_t, nr_vnodes);
     vnuma->vmemrange = xmalloc_array(xen_vmemrange_t, nr_ranges);
 
     if ( vnuma->vdistance == NULL || vnuma->vmemrange == NULL ||
@@ -382,30 +382,40 @@ static struct vnuma_info *vnuma_init(con
                          nr_vnodes * nr_vnodes) )
         goto vnuma_fail;
 
+    if ( copy_from_guest(info->vmemrange, uinfo->vmemrange,
+                         uinfo->nr_vmemranges) )
+        goto vnuma_fail;
+
     if ( copy_from_guest(info->vcpu_to_vnode, uinfo->vcpu_to_vnode,
                          d->max_vcpus) )
         goto vnuma_fail;
 
-    if ( copy_from_guest(info->vnode_to_pnode, uinfo->vnode_to_pnode,
-                         nr_vnodes) )
-        goto vnuma_fail;
+    ret = -E2BIG;
+    for ( i = 0; i < d->max_vcpus; ++i )
+        if ( info->vcpu_to_vnode[i] >= nr_vnodes )
+            goto vnuma_fail;
 
-    if (copy_from_guest(info->vmemrange, uinfo->vmemrange,
-                        uinfo->nr_vmemranges))
-        goto vnuma_fail;
+    for ( i = 0; i < nr_vnodes; ++i )
+    {
+        unsigned int pnode;
+
+        ret = -EFAULT;
+        if ( copy_from_guest_offset(&pnode, uinfo->vnode_to_pnode, i, 1) )
+            goto vnuma_fail;
+        ret = -E2BIG;
+        if ( pnode >= MAX_NUMNODES )
+            goto vnuma_fail;
+        info->vnode_to_pnode[i] = pnode;
+    }
 
     info->nr_vnodes = nr_vnodes;
     info->nr_vmemranges = uinfo->nr_vmemranges;
 
     /* Check that vmemranges flags are zero. */
+    ret = -EINVAL;
     for ( i = 0; i < info->nr_vmemranges; i++ )
-    {
         if ( info->vmemrange[i].flags != 0 )
-        {
-            ret = -EINVAL;
             goto vnuma_fail;
-        }
-    }
 
     return info;
 
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -4,6 +4,7 @@
 
 #include <public/xen.h>
 #include <asm/domain.h>
+#include <asm/numa.h>
 
 typedef union {
     struct vcpu_guest_context *nat;
@@ -99,7 +100,7 @@ struct vnuma_info {
     unsigned int nr_vmemranges;
     unsigned int *vdistance;
     unsigned int *vcpu_to_vnode;
-    unsigned int *vnode_to_pnode;
+    nodeid_t *vnode_to_pnode;
     struct xen_vmemrange *vmemrange;
 };
 






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------030508040400070006090404-- --===============6335061490371407491== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============6335061490371407491==--