Messages in this thread | ![/](/images/icornerl.gif) | | Date | Mon, 25 Dec 2006 10:55:51 +0200 | From | Muli Ben-Yehuda <> | Subject | Re: [PATCH -mm] ehca: avoid crash on kthread_create() failure |
| |
On Mon, Dec 25, 2006 at 05:30:23PM +0900, Akinobu Mita wrote: > On Mon, Dec 25, 2006 at 05:12:57PM +0900, Akinobu Mita wrote: > > Index: 2.6-mm/drivers/infiniband/hw/ehca/ehca_irq.c > > =================================================================== > > --- 2.6-mm.orig/drivers/infiniband/hw/ehca/ehca_irq.c > > +++ 2.6-mm/drivers/infiniband/hw/ehca/ehca_irq.c > > @@ -606,13 +606,16 @@ static int comp_task(void *__cct) > > static struct task_struct *create_comp_task(struct ehca_comp_pool *pool, > > int cpu) > > { > > + struct task_struct *task; > > struct ehca_cpu_comp_task *cct; > > > > cct = per_cpu_ptr(pool->cpu_comp_tasks, cpu); > > spin_lock_init(&cct->task_lock); > > INIT_LIST_HEAD(&cct->cq_list); > > init_waitqueue_head(&cct->wait_queue); > > - cct->task = kthread_create(comp_task, cct, "ehca_comp/%d", cpu); > > + task = kthread_create(comp_task, cct, "ehca_comp/%d", cpu); > > + if (!IS_ERR(task)) > > + cct->task = task; > > > > return cct->task; > > } > > This patch is wrong. It changes to make create_comp_task() return NULL > on failure. > > (BTW, all these ehca fixes are not compile tested due to luck of > cross comile environment) > > Subject: [PATCH -mm] ehca: avoid crash on kthread_create() failure (v2) > > This patch disallows invalid task_struct pointer returned by > kthread_create() to be written to percpu data to avoid crash. > > Cc: Heiko Carstens <heiko.carstens@de.ibm.com> > Cc: Hoang-Nam Nguyen <hnguyen@de.ibm.com> > Cc: Christoph Raisch <raisch@de.ibm.com> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > > --- > drivers/infiniband/hw/ehca/ehca_irq.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > Index: 2.6-mm/drivers/infiniband/hw/ehca/ehca_irq.c > =================================================================== > --- 2.6-mm.orig/drivers/infiniband/hw/ehca/ehca_irq.c > +++ 2.6-mm/drivers/infiniband/hw/ehca/ehca_irq.c > @@ -606,15 +606,18 @@ static int comp_task(void *__cct) > static struct task_struct *create_comp_task(struct ehca_comp_pool *pool, > int cpu) > { > + struct task_struct *task; > struct ehca_cpu_comp_task *cct; > > cct = per_cpu_ptr(pool->cpu_comp_tasks, cpu); > spin_lock_init(&cct->task_lock); > INIT_LIST_HEAD(&cct->cq_list); > init_waitqueue_head(&cct->wait_queue); > - cct->task = kthread_create(comp_task, cct, "ehca_comp/%d", cpu); > + task = kthread_create(comp_task, cct, "ehca_comp/%d", cpu); > + if (!IS_ERR(task)) > + cct->task = task; > > - return cct->task; > + return task; > } > > static void destroy_comp_task(struct ehca_comp_pool *pool, > @@ -684,8 +687,10 @@ static int comp_pool_callback(struct not > case CPU_UP_CANCELED: > ehca_gen_dbg("CPU: %x (CPU_CANCELED)", cpu); > cct = per_cpu_ptr(pool->cpu_comp_tasks, cpu); > - kthread_bind(cct->task, any_online_cpu(cpu_online_map)); > - destroy_comp_task(pool, cpu); > + if (cct->task) { > + kthread_bind(cct->task, any_online_cpu(cpu_online_map)); > + destroy_comp_task(pool, cpu); > + }
This is correct because cct is allocated via alloc_percpu, which in turn calls kzalloc, which means cct->task is NULL by default, but it's a little too obscure for me. How about making it explicit?
task = kthread_create(...) if (!IS_ERR(task)) cct->task = task; else cct->task = NULL;
return cct->task;
Cheers, Muli - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |