From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH 1/2] Fix sched_priv corruption in ARINC653 alloc_vdata. Date: Fri, 1 Nov 2013 13:53:42 +0000 Message-ID: <5273B266.1050504@citrix.com> References: <1383252473-3067-1-git-send-email-nate.studer@dornerworks.com> <1383252473-3067-2-git-send-email-nate.studer@dornerworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1383252473-3067-2-git-send-email-nate.studer@dornerworks.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: Nathan Studer Cc: george.dunlap@eu.citrix.com, smartin@milliways.cl, robert.vanvossen@dornerworks.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 31/10/13 20:47, Nathan Studer wrote: > From: Nathan Studer > > The ARINC653 scheduler was directly assigning and manipulating > the sched_priv field of a vcpu in its alloc_vdata function. > > When creating a cpu pool, this resulted in the corruption > of the sched_priv field of the vcpu, which was then passed > to the initial scheduler's free_vdata function with > disastrous results. > > Signed-off-by: Nathan Studer This looks sane. Reviewed-by: Andrew Cooper You have one line of misc whitespace change. Given the extent of trailing whitespace in the file, it might be worth having a separate patch in the series which fixes all the whitespace at once. Along with that, it would be kind to put a "Local variables" block in as well (see the bottom of sched.h as an example). > --- > xen/common/sched_arinc653.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/xen/common/sched_arinc653.c b/xen/common/sched_arinc653.c > index 2502192..a1d9443 100644 > --- a/xen/common/sched_arinc653.c > +++ b/xen/common/sched_arinc653.c > @@ -380,11 +380,14 @@ a653sched_deinit(const struct scheduler *ops) > static void * > a653sched_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd) > { > + arinc653_vcpu_t *svc; > + > /* > * Allocate memory for the ARINC 653-specific scheduler data information > * associated with the given VCPU (vc). > - */ > - if ( (vc->sched_priv = xmalloc(arinc653_vcpu_t)) == NULL ) > + */ > + svc = xmalloc(arinc653_vcpu_t); > + if ( svc == NULL ) > return NULL; > > /* > @@ -393,13 +396,13 @@ a653sched_alloc_vdata(const struct scheduler *ops, struct vcpu *vc, void *dd) > * will call the vcpu_wake scheduler callback function and our scheduler > * will mark the VCPU awake. > */ > - AVCPU(vc)->vc = vc; > - AVCPU(vc)->awake = 0; > + svc->vc = vc; > + svc->awake = 0; > if ( !is_idle_vcpu(vc) ) > - list_add(&AVCPU(vc)->list, &SCHED_PRIV(ops)->vcpu_list); > + list_add(&svc->list, &SCHED_PRIV(ops)->vcpu_list); > update_schedule_vcpus(ops); > > - return AVCPU(vc); > + return svc; > } > > /**