Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756760AbcKKN6y (ORCPT ); Fri, 11 Nov 2016 08:58:54 -0500 Received: from mail-wm0-f67.google.com ([74.125.82.67]:35872 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756745AbcKKN6t (ORCPT ); Fri, 11 Nov 2016 08:58:49 -0500 MIME-Version: 1.0 In-Reply-To: <20161111105601.GJ10219@e106497-lin.cambridge.arm.com> References: <1478853968-25169-1-git-send-email-shailendra.v@samsung.com> <20161111105601.GJ10219@e106497-lin.cambridge.arm.com> From: Emil Velikov Date: Fri, 11 Nov 2016 13:58:46 +0000 Message-ID: Subject: Re: [PATCH] Gpu: drm: arm: - Fix possible dereference of NULL To: Liviu Dudau Cc: Shailendra Verma , vidushi.koul@samsung.com, "Linux-Kernel@Vger. Kernel. Org" , ML dri-devel , Mali DP Maintainers , Shailendra Verma Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1289 Lines: 35 On 11 November 2016 at 10:56, Liviu Dudau wrote: > Hi Shailendra, > > On Fri, Nov 11, 2016 at 02:16:08PM +0530, Shailendra Verma wrote: >> From: "Shailendra Verma" >> >> There is possible dereference of NULL pointer if kmalloc fails. > > You could add: ... when the function returns. From the patch itself it is > not clear where the problem is. > As the function returns we have "return &state->base;" Since base is at offset 0 there will be no deref and the compiler will return NULL. Not sure if that's 100% legal, though. >> --- >> drivers/gpu/drm/arm/malidp_planes.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/arm/malidp_planes.c b/drivers/gpu/drm/arm/malidp_planes.c >> index 82c193e..f769398 100644 >> --- a/drivers/gpu/drm/arm/malidp_planes.c >> +++ b/drivers/gpu/drm/arm/malidp_planes.c >> @@ -54,6 +54,9 @@ struct drm_plane_state *malidp_duplicate_plane_state(struct drm_plane *plane) >> return NULL; >> >> state = kmalloc(sizeof(*state), GFP_KERNEL); >> + if (!state) >> + return NULL; >> + >> if (state) { Might want to drop this line - as-is things read quite weird ? Either way, not my driver - so don't read too much into the above ;-) Emil