Re: [PATCH v5 12/17] xen/libxc: sched: DOMCTL_*vcpuaffinity works with hard and soft affinity

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2955 bytes --]

On mar, 2013-12-03 at 10:06 +0000, Jan Beulich wrote:
> >>> On 03.12.13 at 11:02, "Jan Beulich" <JBeulich@suse.com> wrote:
> >>>> On 02.12.13 at 19:29, Dario Faggioli <dario.faggioli@citrix.com> wrote:
> >> +                goto setvcpuaffinity_out;
> >> +
> >> +            /*
> >> +             * We both set a new affinity and report back to the caller what
> >> +             * the scheduler will be effectively using.
> >> +             */
> >> +            if ( vcpuaff->flags & XEN_VCPUAFFINITY_HARD )
> >> +            {
> >> +                ret = xenctl_bitmap_to_bitmap(cpumask_bits(new_affinity),
> >> +                                              &vcpuaff->cpumap_hard,
> >> +                                              vcpuaff->cpumap_hard.nr_bits);
> > 
> > There's no code above range checking vcpuaff->cpumap_hard.nr_bits,
> > yet xenctl_bitmap_to_bitmap() uses the passed in value to write into
> > the array pointed to by the first argument. Why is this not
> > xenctl_bitmap_to_cpumask() in the first place?
> 
> And just to make it explicit - with fundamental flaws like this, I'm
> not certain anymore whether we really ought to rush this series
> in for 4.4.
> 
Well, I certainly have no intention to say that this isn't something
that I overlooked. Just for the sake of completeness, that's what's
required to fix it:

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 9eecb5e..cd817f3 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -661,7 +661,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             {
                 ret = xenctl_bitmap_to_bitmap(cpumask_bits(new_affinity),
                                               &vcpuaff->cpumap_hard,
-                                              vcpuaff->cpumap_hard.nr_bits);
+                                              nr_cpu_ids);
                 if ( !ret )
                     ret = vcpu_set_hard_affinity(v, new_affinity);
                 if ( ret )
@@ -679,7 +679,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
             {
                 ret = xenctl_bitmap_to_bitmap(cpumask_bits(new_affinity),
                                               &vcpuaff->cpumap_soft,
-                                              vcpuaff->cpumap_soft.nr_bits);
+                                              nr_cpu_ids);
                 if ( !ret)
                     ret = vcpu_set_soft_affinity(v, new_affinity);
                 if ( ret )

That being said, I definitely won't interfere with the decision of
taking it or not for 4.4. Just let me know. :-)

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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