| Message ID | 1568824231-4007-1-git-send-email-allen.pais@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amdkfd: fix a potential NULL pointer dereference | expand |
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);
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); + dev_err(kfd_chardev(), "Failed to allocate KFD IH workqueue\n"); + return kfd->ih_wq; + } spin_lock_init(&kfd->interrupt_lock); INIT_WORK(&kfd->interrupt_work, interrupt_wq);
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(+)