| Message ID | 1467226517-29098-1-git-send-email-imunsie@au.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 30/06/16 04:55, Ian Munsie wrote: > From: Ian Munsie <imunsie@au1.ibm.com> > > If a kernel context is initialised and does not have any AFU interrupts > allocated it will cause a NULL pointer dereference when the context is > detached since the irq_names list will not have been initialised. > > Move the initialisation of the irq_names list into the cxl_context_init > routine so that it will be valid for the entire lifetime of the context > and will not cause a NULL pointer dereference. > > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> As it's nice having your machine not crash on every shutdown... Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
On Thu, 2016-06-30 at 08:28 +1000, Andrew Donnellan wrote: > On 30/06/16 04:55, Ian Munsie wrote: > > > > From: Ian Munsie <imunsie@au1.ibm.com> > > > > If a kernel context is initialised and does not have any AFU interrupts > > allocated it will cause a NULL pointer dereference when the context is > > detached since the irq_names list will not have been initialised. > > > > Move the initialisation of the irq_names list into the cxl_context_init > > routine so that it will be valid for the entire lifetime of the context > > and will not cause a NULL pointer dereference. > > > > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> > As it's nice having your machine not crash on every shutdown... Fixes: ???? cheers
On 30/06/16 15:00, Michael Ellerman wrote: > On Thu, 2016-06-30 at 08:28 +1000, Andrew Donnellan wrote: >> On 30/06/16 04:55, Ian Munsie wrote: >>> >>> From: Ian Munsie <imunsie@au1.ibm.com> >>> >>> If a kernel context is initialised and does not have any AFU interrupts >>> allocated it will cause a NULL pointer dereference when the context is >>> detached since the irq_names list will not have been initialised. >>> >>> Move the initialisation of the irq_names list into the cxl_context_init >>> routine so that it will be valid for the entire lifetime of the context >>> and will not cause a NULL pointer dereference. >>> >>> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> > >> As it's nice having your machine not crash on every shutdown... > > Fixes: ???? Ian can correct me if I'm wrong, but I suspect this doesn't affect cxlflash (the only current user of the cxl kernel API) - this issue was hit while working on CAPI support for mlx5.
Excerpts from andrew.donnellan's message of 2016-06-30 15:15:02 +1000: > On 30/06/16 15:00, Michael Ellerman wrote: > > On Thu, 2016-06-30 at 08:28 +1000, Andrew Donnellan wrote: > >> On 30/06/16 04:55, Ian Munsie wrote: > >>> > >>> From: Ian Munsie <imunsie@au1.ibm.com> > >>> > >>> If a kernel context is initialised and does not have any AFU interrupts > >>> allocated it will cause a NULL pointer dereference when the context is > >>> detached since the irq_names list will not have been initialised. > >>> > >>> Move the initialisation of the irq_names list into the cxl_context_init > >>> routine so that it will be valid for the entire lifetime of the context > >>> and will not cause a NULL pointer dereference. > >>> > >>> Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> > > > >> As it's nice having your machine not crash on every shutdown... > > > > Fixes: ???? > > Ian can correct me if I'm wrong, but I suspect this doesn't affect > cxlflash (the only current user of the cxl kernel API) - this issue was > hit while working on CAPI support for mlx5. Correct - no current user hits this bug, but the upcoming mlx5 support does because of the way it uses interrupts. -Ian
On Wed, 2016-29-06 at 18:55:17 UTC, Ian Munsie wrote: > From: Ian Munsie <imunsie@au1.ibm.com> > > If a kernel context is initialised and does not have any AFU interrupts > allocated it will cause a NULL pointer dereference when the context is > detached since the irq_names list will not have been initialised. > > Move the initialisation of the irq_names list into the cxl_context_init > routine so that it will be valid for the entire lifetime of the context > and will not cause a NULL pointer dereference. > > Signed-off-by: Ian Munsie <imunsie@au1.ibm.com> > Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/f5c9df9a442f586b1839476272 cheers
diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c index 26d206b..edbb99e 100644 --- a/drivers/misc/cxl/context.c +++ b/drivers/misc/cxl/context.c @@ -67,6 +67,8 @@ int cxl_context_init(struct cxl_context *ctx, struct cxl_afu *afu, bool master, ctx->pending_fault = false; ctx->pending_afu_err = false; + INIT_LIST_HEAD(&ctx->irq_names); + /* * When we have to destroy all contexts in cxl_context_detach_all() we * end up with afu_release_irqs() called from inside a diff --git a/drivers/misc/cxl/irq.c b/drivers/misc/cxl/irq.c index 8def455..f3a7d4a 100644 --- a/drivers/misc/cxl/irq.c +++ b/drivers/misc/cxl/irq.c @@ -260,9 +260,6 @@ int afu_allocate_irqs(struct cxl_context *ctx, u32 count) else alloc_count = count + 1; - /* Initialize the list head to hold irq names */ - INIT_LIST_HEAD(&ctx->irq_names); - if ((rc = cxl_ops->alloc_irq_ranges(&ctx->irqs, ctx->afu->adapter, alloc_count))) return rc;