* [PATCH] lpfc: fix memory leak and NULL dereference
@ 2015-09-23 13:32 Sudip Mukherjee
2015-10-08 14:20 ` Sudip Mukherjee
2015-10-12 18:02 ` James Smart
0 siblings, 2 replies; 3+ messages in thread
From: Sudip Mukherjee @ 2015-09-23 13:32 UTC (permalink / raw)
To: James Smart, Dick Kennedy, James E.J. Bottomley
Cc: linux-kernel, linux-scsi, Sudip Mukherjee
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 <sudip@vectorindia.org>
---
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;
--
1.9.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] lpfc: fix memory leak and NULL dereference
2015-09-23 13:32 [PATCH] lpfc: fix memory leak and NULL dereference Sudip Mukherjee
@ 2015-10-08 14:20 ` Sudip Mukherjee
2015-10-12 18:02 ` James Smart
1 sibling, 0 replies; 3+ messages in thread
From: Sudip Mukherjee @ 2015-10-08 14:20 UTC (permalink / raw)
To: James Smart, Dick Kennedy, James E.J. Bottomley; +Cc: linux-kernel, linux-scsi
On Wed, Sep 23, 2015 at 07:02:32PM +0530, 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 <sudip@vectorindia.org>
> ---
A gentle ping.
regards
sudip
>
> 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;
> --
> 1.9.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] lpfc: fix memory leak and NULL dereference
2015-09-23 13:32 [PATCH] lpfc: fix memory leak and NULL dereference Sudip Mukherjee
2015-10-08 14:20 ` Sudip Mukherjee
@ 2015-10-12 18:02 ` James Smart
1 sibling, 0 replies; 3+ messages in thread
From: James Smart @ 2015-10-12 18:02 UTC (permalink / raw)
To: Sudip Mukherjee, Dick Kennedy, James E.J. Bottomley
Cc: linux-kernel, linux-scsi
Looks Good - Thank you Sudip.
Reviewed-by: James Smart <james.smart@avagotech.com>
-- 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 <sudip@vectorindia.org>
> ---
>
> 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;
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-10-12 18:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-23 13:32 [PATCH] lpfc: fix memory leak and NULL dereference Sudip Mukherjee
2015-10-08 14:20 ` Sudip Mukherjee
2015-10-12 18:02 ` James Smart
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).