[Libvir] [PATCH] virsh Range check in Credit Scheduler

[Daniel Veillard]

On Wed, Jul 18, 2007 at 09:11:42AM +0900, Atsushi SAKAI wrote:
> Hi, Daniel
> 
>  uint16(weight and cap) is copyied in src/virsh.c
> so It cannot rewrite on src/xen_internal.c.

   xenHypervisorSetSchedulerParameters() do get the int values
in the case XEN_SCHEDULER_CREDIT :
  op_dom.u.getschedinfo.u.credit.weight = params[i].value.ui;
and
  op_dom.u.getschedinfo.u.credit.cap = params[i].value.ui;

 the ui field is an unsigned long. the test against the value
1 to USHRT_MAX can be done there and the case where one have
a negative value at the virsh level would correspond to an extremely
large integer in xenHypervisorSetSchedulerParameters() after the
unsigned cast.

  See the enclosed patch, please check,

    thanks,

Daniel


-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/
-------------- next part --------------
Index: src/xen_internal.c
===================================================================
RCS file: /data/cvs/libxen/src/xen_internal.c,v
retrieving revision 1.85
diff -u -p -r1.85 xen_internal.c
--- src/xen_internal.c	12 Jul 2007 08:57:52 -0000	1.85
+++ src/xen_internal.c	18 Jul 2007 08:15:01 -0000
@@ -1170,6 +1170,7 @@ xenHypervisorSetSchedulerParameters(virD
 				 virSchedParameterPtr params, int nparams)
 {
     int i;
+    unsigned int val;
     xenUnifiedPrivatePtr priv;
 
     if ((domain == NULL) || (domain->conn == NULL)) {
@@ -1237,11 +1238,25 @@ xenHypervisorSetSchedulerParameters(virD
             for (i = 0; i < nparams; i++) {
                 if (STREQ (params[i].field, str_weight) &&
                     params[i].type == VIR_DOMAIN_SCHED_FIELD_UINT) {
-                    op_dom.u.getschedinfo.u.credit.weight = params[i].value.ui;
+		    val = params[i].value.ui;
+		    if ((val < 1) || (val > USHRT_MAX)) {
+		        virXenErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
+       _("Credit scheduler weight parameter (%d) is out of range (1-65535)"),
+                                         val);
+			return(-1);
+		    }
+                    op_dom.u.getschedinfo.u.credit.weight = val;
 		    weight_set = 1;
 		} else if (STREQ (params[i].field, str_cap) &&
                     params[i].type == VIR_DOMAIN_SCHED_FIELD_UINT) {
-                    op_dom.u.getschedinfo.u.credit.cap = params[i].value.ui;
+		    val = params[i].value.ui;
+		    if (val > USHRT_MAX) {
+		        virXenErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,
+       _("Credit scheduler cap parameter (%d) is out of range (0-65535)"),
+                                         val);
+			return(-1);
+		    }
+                    op_dom.u.getschedinfo.u.credit.cap = val;
 		    cap_set = 1;
 	        } else {
 		    virXenErrorFunc (VIR_ERR_INVALID_ARG, __FUNCTION__,