Messages in this thread | | | Date | Tue, 6 Apr 2010 16:40:38 -0700 | From | Greg KH <> | Subject | Re: patch drm-radeon-kms-fix-null-pointer-dereference-if-memory-allocation-failed.patch added to 2.6.33-stable tree |
| |
On Tue, Apr 06, 2010 at 11:45:38PM +0200, Stefan Lippers-Hollmann wrote: > Hi > > On Tuesday 06 April 2010, gregkh@suse.de wrote: > > This is a note to let you know that we have just queued up the patch titled > > > > Subject: drm/radeon/kms: Fix NULL pointer dereference if memory allocation failed. > > > > to the 2.6.33-stable tree. Its filename is > > > > drm-radeon-kms-fix-null-pointer-dereference-if-memory-allocation-failed.patch > > > > A git repo of this tree can be found at > > http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary > > > > > > From fcbc451ba1948fba967198bd150ecbd10bbb7075 Mon Sep 17 00:00:00 2001 > > From: Pauli Nieminen <suokkos@gmail.com> > > Date: Fri, 19 Mar 2010 07:44:33 +0000 > > Subject: drm/radeon/kms: Fix NULL pointer dereference if memory allocation failed. > > > > From: Pauli Nieminen <suokkos@gmail.com> > > > > commit fcbc451ba1948fba967198bd150ecbd10bbb7075 upstream. > > > > When there is allocation failure in radeon_cs_parser_relocs parser->nrelocs > > is not cleaned. This causes NULL pointer defeference in radeon_cs_parser_fini > > when clean up code is trying to loop over the relocation array and free the > > objects. > > > > Fix adds a check for a possible NULL pointer in clean up code. > [...] > > This patch breaks compiling kernel 2.6.33 + the current stable queue: > > CC [M] drivers/gpu/drm/radeon/radeon_cs.o > /tmp/buildd/linux-sidux-2.6-2.6.33/debian/build/source_amd64_none/drivers/gpu/drm/radeon/radeon_cs.c: In function 'radeon_cs_parser_fini': > /tmp/buildd/linux-sidux-2.6-2.6.33/debian/build/source_amd64_none/drivers/gpu/drm/radeon/radeon_cs.c:200: error: implicit declaration of function 'drm_gem_object_unreference_unlocked' > make[6]: *** [drivers/gpu/drm/radeon/radeon_cs.o] Error 1 > > as it depends on the introduction of drm_gem_object_unreference_unlocked() > in: > > Commit: c3ae90c099bb62387507e86da7cf799850444b08 > Author: Luca Barbieri <luca@luca-barbieri.com> > AuthorDate: Tue Feb 9 05:49:11 2010 +0000 > > drm: introduce drm_gem_object_[handle_]unreference_unlocked > > This patch introduces the drm_gem_object_unreference_unlocked > and drm_gem_object_handle_unreference_unlocked functions that > do not require holding struct_mutex. > > drm_gem_object_unreference_unlocked calls the new > ->gem_free_object_unlocked entry point if available, and > otherwise just takes struct_mutex and just calls ->gem_free_object > > which in turn suggests: > > Commit: bc9025bdc4e2b591734cca17697093845007b63d > Author: Luca Barbieri <luca@luca-barbieri.com> > AuthorDate: Tue Feb 9 05:49:12 2010 +0000 > > Use drm_gem_object_[handle_]unreference_unlocked where possible > > Mostly obvious simplifications. > > The i915 pread/pwrite ioctls, intel_overlay_put_image and > nouveau_gem_new were incorrectly using the locked versions > without locking: this is also fixed in this patch. > > which don't really look like candidates for 2.6.33-stable. > > > --- a/drivers/gpu/drm/radeon/radeon_cs.c > > +++ b/drivers/gpu/drm/radeon/radeon_cs.c > > @@ -193,11 +193,13 @@ static void radeon_cs_parser_fini(struct > > radeon_bo_list_fence(&parser->validated, parser->ib->fence); > > } > > radeon_bo_list_unreserve(&parser->validated); > > - for (i = 0; i < parser->nrelocs; i++) { > > - if (parser->relocs[i].gobj) { > > - mutex_lock(&parser->rdev->ddev->struct_mutex); > > - drm_gem_object_unreference(parser->relocs[i].gobj); > > - mutex_unlock(&parser->rdev->ddev->struct_mutex); > > + if (parser->relocs != NULL) { > ^ the only important part, the rest merely covers the new indentation > level > > > + for (i = 0; i < parser->nrelocs; i++) { > > + if (parser->relocs[i].gobj) { > > + mutex_lock(&parser->rdev->ddev->struct_mutex); > > + drm_gem_object_unreference_unlocked(parser->relocs[i].gobj); > ^ drm_gem_object_unreference_unlocked() doesn't exist in 2.6.33, yet > we can use drm_gem_object_unreference() instead. > > > + mutex_unlock(&parser->rdev->ddev->struct_mutex); > > + } > > } > > } > > kfree(parser->track); > > As a consequence, I'd suggest to merely backport the NULL pointer check, > while ignoring the simplification of using the newly introduced > drm_gem_object_unreference_unlocked() from 2.6.34: > > Signed-off-by: Stefan Lippers-Hollmann <s.l-h@gmx.de>
Very cool, thanks for doing this, I just noticed it failing to build here as well when you sent this.
I've queued up your version as it makes more sense.
thanks,
greg k-h
| |