* [PATCH] gpu: radeon: fix a potential NULL-pointer dereference @ 2019-03-23 2:29 Kangjie Lu 2019-03-25 10:32 ` Michel Dänzer 0 siblings, 1 reply; 4+ messages in thread From: Kangjie Lu @ 2019-03-23 2:29 UTC (permalink / raw) To: kjlu Cc: pakki001, Alex Deucher, Christian König, David (ChunMing) Zhou, David Airlie, Daniel Vetter, amd-gfx, dri-devel, linux-kernel In case alloc_workqueue fails, the fix frees memory and returns to avoid potential NULL pointer dereference. Signed-off-by: Kangjie Lu <kjlu@umn.edu> --- drivers/gpu/drm/radeon/radeon_display.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index aa898c699101..a31305755a77 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -678,6 +678,11 @@ static void radeon_crtc_init(struct drm_device *dev, int index) drm_mode_crtc_set_gamma_size(&radeon_crtc->base, 256); radeon_crtc->crtc_id = index; radeon_crtc->flip_queue = alloc_workqueue("radeon-crtc", WQ_HIGHPRI, 0); + if (!radeon_crtc->flip_queue) { + DRM_ERROR("failed to allocate the flip queue\n"); + kfree(radeon_crtc); + return; + } rdev->mode_info.crtcs[index] = radeon_crtc; if (rdev->family >= CHIP_BONAIRE) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gpu: radeon: fix a potential NULL-pointer dereference 2019-03-23 2:29 [PATCH] gpu: radeon: fix a potential NULL-pointer dereference Kangjie Lu @ 2019-03-25 10:32 ` Michel Dänzer 2019-03-25 21:05 ` [PATCH v2] " Kangjie Lu 0 siblings, 1 reply; 4+ messages in thread From: Michel Dänzer @ 2019-03-25 10:32 UTC (permalink / raw) To: Kangjie Lu Cc: David Airlie, linux-kernel, amd-gfx, dri-devel, Daniel Vetter, pakki001, Alex Deucher, Christian König Hi Kangjie, thanks for your patch. On 2019-03-23 3:29 a.m., Kangjie Lu wrote: > In case alloc_workqueue fails, the fix frees memory and > returns to avoid potential NULL pointer dereference. > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > drivers/gpu/drm/radeon/radeon_display.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c > index aa898c699101..a31305755a77 100644 > --- a/drivers/gpu/drm/radeon/radeon_display.c > +++ b/drivers/gpu/drm/radeon/radeon_display.c > @@ -678,6 +678,11 @@ static void radeon_crtc_init(struct drm_device *dev, int index) > drm_mode_crtc_set_gamma_size(&radeon_crtc->base, 256); > radeon_crtc->crtc_id = index; > radeon_crtc->flip_queue = alloc_workqueue("radeon-crtc", WQ_HIGHPRI, 0); > + if (!radeon_crtc->flip_queue) { > + DRM_ERROR("failed to allocate the flip queue\n"); > + kfree(radeon_crtc); This would leak some memory referenced by struct drm_crtc. To solve this, I suggest calling radeon_crtc_destroy here and making that cope with radeon_crtc->flip_queue being NULL. Also, I'm not sure all driver code can handle some CRTCs not initializing. Given that, and as alloc_workqueue presumably only fails if the system is essentially out of memory anyway, it's probably better for radeon_crtc_init to return -ENOMEM in this case and for radeon_modeset_init to propagate that, which will prevent the driver as a whole from initializing. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] gpu: radeon: fix a potential NULL-pointer dereference 2019-03-25 10:32 ` Michel Dänzer @ 2019-03-25 21:05 ` Kangjie Lu 2019-03-26 9:51 ` Michel Dänzer 0 siblings, 1 reply; 4+ messages in thread From: Kangjie Lu @ 2019-03-25 21:05 UTC (permalink / raw) To: kjlu Cc: pakki001, Alex Deucher, Christian König, David (ChunMing) Zhou, David Airlie, Daniel Vetter, amd-gfx, dri-devel, linux-kernel In case alloc_workqueue fails, the fix frees memory and returns -ENOMEM to avoid potential NULL pointer dereference. Signed-off-by: Kangjie Lu <kjlu@umn.edu> --- v2: use radeon_crtc_destroy to properly clean up resources as suggested by Michel Dänzer <michel@daenzer.net> --- drivers/gpu/drm/radeon/radeon_display.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index aa898c699101..3c6d3289316b 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -663,7 +663,7 @@ static const struct drm_crtc_funcs radeon_crtc_funcs = { .page_flip_target = radeon_crtc_page_flip_target, }; -static void radeon_crtc_init(struct drm_device *dev, int index) +static int radeon_crtc_init(struct drm_device *dev, int index) { struct radeon_device *rdev = dev->dev_private; struct radeon_crtc *radeon_crtc; @@ -671,13 +671,18 @@ static void radeon_crtc_init(struct drm_device *dev, int index) radeon_crtc = kzalloc(sizeof(struct radeon_crtc) + (RADEONFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL); if (radeon_crtc == NULL) - return; + return -ENOMEM; drm_crtc_init(dev, &radeon_crtc->base, &radeon_crtc_funcs); drm_mode_crtc_set_gamma_size(&radeon_crtc->base, 256); radeon_crtc->crtc_id = index; radeon_crtc->flip_queue = alloc_workqueue("radeon-crtc", WQ_HIGHPRI, 0); + if (!radeon_crtc->flip_queue) { + DRM_ERROR("failed to allocate the flip queue\n"); + radeon_crtc_destroy(&radeon_crtc->base); + return -ENOMEM; + } rdev->mode_info.crtcs[index] = radeon_crtc; if (rdev->family >= CHIP_BONAIRE) { @@ -706,6 +711,8 @@ static void radeon_crtc_init(struct drm_device *dev, int index) radeon_atombios_init_crtc(dev, radeon_crtc); else radeon_legacy_init_crtc(dev, radeon_crtc); + + return 0; } static const char *encoder_names[38] = { @@ -1612,7 +1619,9 @@ int radeon_modeset_init(struct radeon_device *rdev) /* allocate crtcs */ for (i = 0; i < rdev->num_crtc; i++) { - radeon_crtc_init(rdev->ddev, i); + ret = radeon_crtc_init(rdev->ddev, i); + if (ret) + return ret; } /* okay we should have all the bios connectors */ -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] gpu: radeon: fix a potential NULL-pointer dereference 2019-03-25 21:05 ` [PATCH v2] " Kangjie Lu @ 2019-03-26 9:51 ` Michel Dänzer 0 siblings, 0 replies; 4+ messages in thread From: Michel Dänzer @ 2019-03-26 9:51 UTC (permalink / raw) To: Kangjie Lu Cc: David (ChunMing) Zhou, David Airlie, linux-kernel, amd-gfx, dri-devel, Daniel Vetter, pakki001, Alex Deucher, Christian König On 2019-03-25 10:05 p.m., Kangjie Lu wrote: > In case alloc_workqueue fails, the fix frees memory and > returns -ENOMEM to avoid potential NULL pointer dereference. > > Signed-off-by: Kangjie Lu <kjlu@umn.edu> > --- > v2: use radeon_crtc_destroy to properly clean up resources as > suggested by Michel Dänzer <michel@daenzer.net> > > [...] > > @@ -671,13 +671,18 @@ static void radeon_crtc_init(struct drm_device *dev, int index) > > radeon_crtc = kzalloc(sizeof(struct radeon_crtc) + (RADEONFB_CONN_LIMIT * sizeof(struct drm_connector *)), GFP_KERNEL); > if (radeon_crtc == NULL) > - return; > + return -ENOMEM; > > drm_crtc_init(dev, &radeon_crtc->base, &radeon_crtc_funcs); > > drm_mode_crtc_set_gamma_size(&radeon_crtc->base, 256); > radeon_crtc->crtc_id = index; > radeon_crtc->flip_queue = alloc_workqueue("radeon-crtc", WQ_HIGHPRI, 0); > + if (!radeon_crtc->flip_queue) { > + DRM_ERROR("failed to allocate the flip queue\n"); > + radeon_crtc_destroy(&radeon_crtc->base); > + return -ENOMEM; > + } radeon_crtc_destroy currently unconditionally calls: destroy_workqueue(radeon_crtc->flip_queue); AFAICT destroy_workqueue will blow up if NULL is passed to it, so radeon_crtc_destroy needs to check for that. -- Earthling Michel Dänzer | https://www.amd.com Libre software enthusiast | Mesa and X developer ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-03-26 9:51 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-03-23 2:29 [PATCH] gpu: radeon: fix a potential NULL-pointer dereference Kangjie Lu 2019-03-25 10:32 ` Michel Dänzer 2019-03-25 21:05 ` [PATCH v2] " Kangjie Lu 2019-03-26 9:51 ` Michel Dänzer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).