From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@eu.citrix.com>
Subject: Re: [PATCH v5 12/17] xen/libxc: sched:
 DOMCTL_*vcpuaffinity works with hard and soft affinity
Date: Tue, 3 Dec 2013 18:21:32 +0000
Message-ID: <529E212C.8070205@eu.citrix.com>
References: <20131202180129.29026.81543.stgit@Solace>
	<20131202182908.29026.23720.stgit@Solace>
	<529DBA3F02000078001093CF@nat28.tlf.novell.com>
	<529DBB4A02000078001093E5@nat28.tlf.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta4.messagelabs.com ([85.158.143.247])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <George.Dunlap@citrix.com>) id 1VnubK-00071l-AY
	for xen-devel@lists.xenproject.org; Tue, 03 Dec 2013 18:21:38 +0000
In-Reply-To: <529DBB4A02000078001093E5@nat28.tlf.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>, Dario Faggioli <dario.faggioli@citrix.com>
Cc: Marcus Granado <Marcus.Granado@eu.citrix.com>, Justin Weaver <jtweaver@hawaii.edu>, Ian Campbell <Ian.Campbell@citrix.com>, Li Yechen <lccycc123@gmail.com>, Andrew Cooper <Andrew.Cooper3@citrix.com>, Juergen Gross <juergen.gross@ts.fujitsu.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Matt Wilson <msw@amazon.com>, xen-devel <xen-devel@lists.xenproject.org>, Keir Fraser <keir@xen.org>, Elena Ufimtseva <ufimtseva@gmail.com>
List-Id: xen-devel@lists.xenproject.org

On 12/03/2013 10:06 AM, 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.

I'm certainly getting nervous about the prospect.  However, the above 
bug would only be triggered by bad input from domain 0, right? I suppose 
even that would be a potential security issue in a highly disaggregated 
environment.

Other bugs in this patch would be similar.  This path is taken on domain 
creation IIUC; so bugs in this particular patch would probably either be 
unexpected behavior of the affinities, or failure to handle unusual 
input from a trusted source (domain 0).

  -George