From: James Smart <jsmart2021@gmail.com>
To: linux-nvme@lists.infradead.org
Cc: axboe@kernel.dk, James Smart <jsmart2021@gmail.com>,
Dick Kennedy <dick.kennedy@broadcom.com>,
martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
jejb@linux.ibm.com, kernel-janitors@vger.kernel.org,
hch@infradead.org, paul.ely@broadcom.com, hare@suse.de,
dan.carpenter@oracle.com
Subject: [PATCH 2/3] lpfc: fix axchg pointer reference after free and double frees
Date: Wed, 20 May 2020 18:59:28 +0000 [thread overview]
Message-ID: <20200520185929.48779-3-jsmart2021@gmail.com> (raw)
In-Reply-To: <20200520185929.48779-1-jsmart2021@gmail.com>
The axchg structure is a structure allocated early in the
lpfc_nvme_unsol_ls_handler() to represent the newly received exchange.
Upon error, the out_fail path in the routine unconditionally frees the
pointer, yet subsequently passes the pointer to the abort routine.
Additionally, the abort routine, lpfc_nvme_unsol_ls_issue_abort(), also
has a failure path that will attempt to delete the pointer on error.
Fix these errors by:
- Removing the unconditional free so that it stays valid if passed
to the abort routine.
- Revise the abort routine to not free the pointer. Instead, return
a success/failure status. Note: if success, the later completion of
the abort frees the structure.
- Back in the unsol_ls_handler() error path, if the abort routine was
skipped (thus no possible reference) or the abort routine returned
error, free the pointer.
Fixes: 3a8070c567aa ("lpfc: Refactor NVME LS receive handling")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
drivers/scsi/lpfc/lpfc_nvmet.c | 3 +--
drivers/scsi/lpfc/lpfc_sli.c | 10 ++++++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index bccf9da302ee..32eb5e873e9b 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -3598,10 +3598,9 @@ lpfc_nvme_unsol_ls_issue_abort(struct lpfc_hba *phba,
abts_wqeq->context2 = NULL;
abts_wqeq->context3 = NULL;
lpfc_sli_release_iocbq(phba, abts_wqeq);
- kfree(ctxp);
lpfc_printf_log(phba, KERN_ERR, LOG_NVME_ABTS,
"6056 Failed to Issue ABTS. Status x%x\n", rc);
- return 0;
+ return 1;
}
/**
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 1aaf40081e21..9e21c4f3b009 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -2813,7 +2813,7 @@ lpfc_nvme_unsol_ls_handler(struct lpfc_hba *phba, struct lpfc_iocbq *piocb)
struct lpfc_async_xchg_ctx *axchg = NULL;
char *failwhy = NULL;
uint32_t oxid, sid, did, fctl, size;
- int ret;
+ int ret = 1;
d_buf = piocb->context2;
@@ -2897,14 +2897,16 @@ lpfc_nvme_unsol_ls_handler(struct lpfc_hba *phba, struct lpfc_iocbq *piocb)
(phba->nvmet_support) ? "T" : "I", ret);
out_fail:
- kfree(axchg);
/* recycle receive buffer */
lpfc_in_buf_free(phba, &nvmebuf->dbuf);
/* If start of new exchange, abort it */
- if (fctl & FC_FC_FIRST_SEQ && !(fctl & FC_FC_EX_CTX))
- lpfc_nvme_unsol_ls_issue_abort(phba, axchg, sid, oxid);
+ if (axchg && (fctl & FC_FC_FIRST_SEQ && !(fctl & FC_FC_EX_CTX)))
+ ret = lpfc_nvme_unsol_ls_issue_abort(phba, axchg, sid, oxid);
+
+ if (ret)
+ kfree(axchg);
}
/**
--
2.26.1
WARNING: multiple messages have this Message-ID (diff)
From: James Smart <jsmart2021@gmail.com>
To: linux-nvme@lists.infradead.org
Cc: axboe@kernel.dk, James Smart <jsmart2021@gmail.com>,
Dick Kennedy <dick.kennedy@broadcom.com>,
martin.petersen@oracle.com, linux-scsi@vger.kernel.org,
jejb@linux.ibm.com, kernel-janitors@vger.kernel.org,
hch@infradead.org, paul.ely@broadcom.com, hare@suse.de,
dan.carpenter@oracle.com
Subject: [PATCH 2/3] lpfc: fix axchg pointer reference after free and double frees
Date: Wed, 20 May 2020 11:59:28 -0700 [thread overview]
Message-ID: <20200520185929.48779-3-jsmart2021@gmail.com> (raw)
In-Reply-To: <20200520185929.48779-1-jsmart2021@gmail.com>
The axchg structure is a structure allocated early in the
lpfc_nvme_unsol_ls_handler() to represent the newly received exchange.
Upon error, the out_fail path in the routine unconditionally frees the
pointer, yet subsequently passes the pointer to the abort routine.
Additionally, the abort routine, lpfc_nvme_unsol_ls_issue_abort(), also
has a failure path that will attempt to delete the pointer on error.
Fix these errors by:
- Removing the unconditional free so that it stays valid if passed
to the abort routine.
- Revise the abort routine to not free the pointer. Instead, return
a success/failure status. Note: if success, the later completion of
the abort frees the structure.
- Back in the unsol_ls_handler() error path, if the abort routine was
skipped (thus no possible reference) or the abort routine returned
error, free the pointer.
Fixes: 3a8070c567aa ("lpfc: Refactor NVME LS receive handling")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
drivers/scsi/lpfc/lpfc_nvmet.c | 3 +--
drivers/scsi/lpfc/lpfc_sli.c | 10 ++++++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index bccf9da302ee..32eb5e873e9b 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -3598,10 +3598,9 @@ lpfc_nvme_unsol_ls_issue_abort(struct lpfc_hba *phba,
abts_wqeq->context2 = NULL;
abts_wqeq->context3 = NULL;
lpfc_sli_release_iocbq(phba, abts_wqeq);
- kfree(ctxp);
lpfc_printf_log(phba, KERN_ERR, LOG_NVME_ABTS,
"6056 Failed to Issue ABTS. Status x%x\n", rc);
- return 0;
+ return 1;
}
/**
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 1aaf40081e21..9e21c4f3b009 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -2813,7 +2813,7 @@ lpfc_nvme_unsol_ls_handler(struct lpfc_hba *phba, struct lpfc_iocbq *piocb)
struct lpfc_async_xchg_ctx *axchg = NULL;
char *failwhy = NULL;
uint32_t oxid, sid, did, fctl, size;
- int ret;
+ int ret = 1;
d_buf = piocb->context2;
@@ -2897,14 +2897,16 @@ lpfc_nvme_unsol_ls_handler(struct lpfc_hba *phba, struct lpfc_iocbq *piocb)
(phba->nvmet_support) ? "T" : "I", ret);
out_fail:
- kfree(axchg);
/* recycle receive buffer */
lpfc_in_buf_free(phba, &nvmebuf->dbuf);
/* If start of new exchange, abort it */
- if (fctl & FC_FC_FIRST_SEQ && !(fctl & FC_FC_EX_CTX))
- lpfc_nvme_unsol_ls_issue_abort(phba, axchg, sid, oxid);
+ if (axchg && (fctl & FC_FC_FIRST_SEQ && !(fctl & FC_FC_EX_CTX)))
+ ret = lpfc_nvme_unsol_ls_issue_abort(phba, axchg, sid, oxid);
+
+ if (ret)
+ kfree(axchg);
}
/**
--
2.26.1
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
WARNING: multiple messages have this Message-ID (diff)
From: James Smart <jsmart2021@gmail.com>
To: linux-nvme@lists.infradead.org
Cc: linux-scsi@vger.kernel.org, kernel-janitors@vger.kernel.org,
paul.ely@broadcom.com, hare@suse.de, jejb@linux.ibm.com,
axboe@kernel.dk, martin.petersen@oracle.com, hch@infradead.org,
dan.carpenter@oracle.com, James Smart <jsmart2021@gmail.com>,
Dick Kennedy <dick.kennedy@broadcom.com>
Subject: [PATCH 2/3] lpfc: fix axchg pointer reference after free and double frees
Date: Wed, 20 May 2020 11:59:28 -0700 [thread overview]
Message-ID: <20200520185929.48779-3-jsmart2021@gmail.com> (raw)
In-Reply-To: <20200520185929.48779-1-jsmart2021@gmail.com>
The axchg structure is a structure allocated early in the
lpfc_nvme_unsol_ls_handler() to represent the newly received exchange.
Upon error, the out_fail path in the routine unconditionally frees the
pointer, yet subsequently passes the pointer to the abort routine.
Additionally, the abort routine, lpfc_nvme_unsol_ls_issue_abort(), also
has a failure path that will attempt to delete the pointer on error.
Fix these errors by:
- Removing the unconditional free so that it stays valid if passed
to the abort routine.
- Revise the abort routine to not free the pointer. Instead, return
a success/failure status. Note: if success, the later completion of
the abort frees the structure.
- Back in the unsol_ls_handler() error path, if the abort routine was
skipped (thus no possible reference) or the abort routine returned
error, free the pointer.
Fixes: 3a8070c567aa ("lpfc: Refactor NVME LS receive handling")
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Dick Kennedy <dick.kennedy@broadcom.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
---
drivers/scsi/lpfc/lpfc_nvmet.c | 3 +--
drivers/scsi/lpfc/lpfc_sli.c | 10 ++++++----
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/lpfc/lpfc_nvmet.c b/drivers/scsi/lpfc/lpfc_nvmet.c
index bccf9da302ee..32eb5e873e9b 100644
--- a/drivers/scsi/lpfc/lpfc_nvmet.c
+++ b/drivers/scsi/lpfc/lpfc_nvmet.c
@@ -3598,10 +3598,9 @@ lpfc_nvme_unsol_ls_issue_abort(struct lpfc_hba *phba,
abts_wqeq->context2 = NULL;
abts_wqeq->context3 = NULL;
lpfc_sli_release_iocbq(phba, abts_wqeq);
- kfree(ctxp);
lpfc_printf_log(phba, KERN_ERR, LOG_NVME_ABTS,
"6056 Failed to Issue ABTS. Status x%x\n", rc);
- return 0;
+ return 1;
}
/**
diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 1aaf40081e21..9e21c4f3b009 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -2813,7 +2813,7 @@ lpfc_nvme_unsol_ls_handler(struct lpfc_hba *phba, struct lpfc_iocbq *piocb)
struct lpfc_async_xchg_ctx *axchg = NULL;
char *failwhy = NULL;
uint32_t oxid, sid, did, fctl, size;
- int ret;
+ int ret = 1;
d_buf = piocb->context2;
@@ -2897,14 +2897,16 @@ lpfc_nvme_unsol_ls_handler(struct lpfc_hba *phba, struct lpfc_iocbq *piocb)
(phba->nvmet_support) ? "T" : "I", ret);
out_fail:
- kfree(axchg);
/* recycle receive buffer */
lpfc_in_buf_free(phba, &nvmebuf->dbuf);
/* If start of new exchange, abort it */
- if (fctl & FC_FC_FIRST_SEQ && !(fctl & FC_FC_EX_CTX))
- lpfc_nvme_unsol_ls_issue_abort(phba, axchg, sid, oxid);
+ if (axchg && (fctl & FC_FC_FIRST_SEQ && !(fctl & FC_FC_EX_CTX)))
+ ret = lpfc_nvme_unsol_ls_issue_abort(phba, axchg, sid, oxid);
+
+ if (ret)
+ kfree(axchg);
}
/**
--
2.26.1
next prev parent reply other threads:[~2020-05-20 18:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-20 18:59 [PATCH 0/3] lpfc: Fix errors in LS receive refactoring James Smart
2020-05-20 18:59 ` James Smart
2020-05-20 18:59 ` James Smart
2020-05-20 18:59 ` [PATCH 1/3] lpfc: Fix pointer checks and comments " James Smart
2020-05-20 18:59 ` James Smart
2020-05-20 18:59 ` James Smart
2020-05-20 18:59 ` James Smart [this message]
2020-05-20 18:59 ` [PATCH 2/3] lpfc: fix axchg pointer reference after free and double frees James Smart
2020-05-20 18:59 ` James Smart
2020-05-20 18:59 ` [PATCH 3/3] lpfc: Fix return value in __lpfc_nvme_ls_abort James Smart
2020-05-20 18:59 ` James Smart
2020-05-20 18:59 ` James Smart
2020-05-22 7:29 ` [PATCH 0/3] lpfc: Fix errors in LS receive refactoring Dan Carpenter
2020-05-22 7:29 ` Dan Carpenter
2020-05-22 7:29 ` Dan Carpenter
2020-05-26 14:53 ` Christoph Hellwig
2020-05-26 14:53 ` Christoph Hellwig
2020-05-26 14:53 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200520185929.48779-3-jsmart2021@gmail.com \
--to=jsmart2021@gmail.com \
--cc=axboe@kernel.dk \
--cc=dan.carpenter@oracle.com \
--cc=dick.kennedy@broadcom.com \
--cc=hare@suse.de \
--cc=hch@infradead.org \
--cc=jejb@linux.ibm.com \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=paul.ely@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.