From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752231AbbJLSCg (ORCPT ); Mon, 12 Oct 2015 14:02:36 -0400 Received: from mail-pa0-f54.google.com ([209.85.220.54]:36161 "EHLO mail-pa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751231AbbJLSCe (ORCPT ); Mon, 12 Oct 2015 14:02:34 -0400 Subject: Re: [PATCH] lpfc: fix memory leak and NULL dereference To: Sudip Mukherjee , Dick Kennedy , "James E.J. Bottomley" References: <1443015152-12301-1-git-send-email-sudipm.mukherjee@gmail.com> Cc: linux-kernel@vger.kernel.org, linux-scsi@vger.kernel.org From: James Smart Message-ID: <561BF5B5.4080901@avagotech.com> Date: Mon, 12 Oct 2015 11:02:29 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1443015152-12301-1-git-send-email-sudipm.mukherjee@gmail.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Looks Good - Thank you Sudip. Reviewed-by: James Smart -- james s On 9/23/2015 6:32 AM, Sudip Mukherjee wrote: > kmalloc() can return NULL and without checking we were dereferencing it. > Moreover if kmalloc succeeds but the function fails in other parts then > we were returning the error code but we missed freeing lcb_context. > While at it fixed one related checkpatch warning. > > Signed-off-by: Sudip Mukherjee > --- > > I am not exactly sure if LSRJT_UNABLE_TPC is the right error code here. > But that was my best guess. > > drivers/scsi/lpfc/lpfc_els.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/lpfc/lpfc_els.c b/drivers/scsi/lpfc/lpfc_els.c > index 36bf58b..a27efd9 100644 > --- a/drivers/scsi/lpfc/lpfc_els.c > +++ b/drivers/scsi/lpfc/lpfc_els.c > @@ -5209,7 +5209,6 @@ lpfc_els_rcv_lcb(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb, > rjt_err = LSRJT_CMD_UNSUPPORTED; > goto rjt; > } > - lcb_context = kmalloc(sizeof(struct lpfc_lcb_context), GFP_KERNEL); > > if (phba->hba_flag & HBA_FCOE_MODE) { > rjt_err = LSRJT_CMD_UNSUPPORTED; > @@ -5240,6 +5239,12 @@ lpfc_els_rcv_lcb(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb, > goto rjt; > } > > + lcb_context = kmalloc(sizeof(*lcb_context), GFP_KERNEL); > + if (!lcb_context) { > + rjt_err = LSRJT_UNABLE_TPC; > + goto rjt; > + } > + > state = (beacon->lcb_sub_command == LPFC_LCB_ON) ? 1 : 0; > lcb_context->sub_command = beacon->lcb_sub_command; > lcb_context->type = beacon->lcb_type; > @@ -5250,6 +5255,7 @@ lpfc_els_rcv_lcb(struct lpfc_vport *vport, struct lpfc_iocbq *cmdiocb, > if (lpfc_sli4_set_beacon(vport, lcb_context, state)) { > lpfc_printf_vlog(ndlp->vport, KERN_ERR, > LOG_ELS, "0193 failed to send mail box"); > + kfree(lcb_context); > lpfc_nlp_put(ndlp); > rjt_err = LSRJT_UNABLE_TPC; > goto rjt;