Messages in this thread | | | From | "Kuehling, Felix" <> | Subject | Re: [PATCH] drm/amdkfd: fix a potential NULL pointer dereference | Date | Thu, 19 Sep 2019 20:38:36 +0000 |
| |
On 2019-09-18 12:30 p.m., Allen Pais wrote: > alloc_workqueue is not checked for errors and as a result, > a potential NULL dereference could occur. > > Signed-off-by: Allen Pais <allen.pais@oracle.com> > --- > drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > index c56ac47..caa82a8 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_interrupt.c > @@ -62,6 +62,11 @@ int kfd_interrupt_init(struct kfd_dev *kfd) > } > > kfd->ih_wq = alloc_workqueue("KFD IH", WQ_HIGHPRI, 1); > + if (unlikely(!kfd->ih_wq)) { > + fifo_free(&kfd->ih_fifo);
This does not compile. I think this should be kfifo_free.
> + dev_err(kfd_chardev(), "Failed to allocate KFD IH workqueue\n"); > + return kfd->ih_wq;
This throws a compiler warning "return makes integer from pointer without a cast". What's worse, kfd->ih_wq is NULL here and kfd_interrupt_init returns 0 to mean success, so returning a NULL pointer is definitely not what you want here. This function should return a negative error code on failure. I propose -ENOMEM.
I'm going to apply your patch with those fixes.
Regards, Felix
> + } > spin_lock_init(&kfd->interrupt_lock); > > INIT_WORK(&kfd->interrupt_work, interrupt_wq);
| |