diff mbox series

[v2,2/2] lightnvm: pblk: Ensure that bio is not freed on recovery

Message ID 20181207082550.10409-2-igor.j.konopko@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,1/2] lightnvm: pblk: Do not overwrite ppa list with meta list | expand

Commit Message

Igor Konopko Dec. 7, 2018, 8:25 a.m. UTC
When we are using PBLK with 0 sized metadata during recovery
process we need to reference a last page of bio. Currently
KASAN reports use-after-free in that case, since bio is
freed on IO completion.

This patch adds addtional bio reference to ensure, that we
can still use bio memory after IO completion. It also ensures
that we are not reusing the same bio on retry_rq path.

Reported-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
---
 drivers/lightnvm/pblk-recovery.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Matias Bjorling Dec. 7, 2018, 12:01 p.m. UTC | #1
On 12/07/2018 09:25 AM, Igor Konopko wrote:
> When we are using PBLK with 0 sized metadata during recovery
> process we need to reference a last page of bio. Currently
> KASAN reports use-after-free in that case, since bio is
> freed on IO completion.
> 
> This patch adds addtional bio reference to ensure, that we
> can still use bio memory after IO completion. It also ensures
> that we are not reusing the same bio on retry_rq path.
> 
> Reported-by: Hans Holmberg <hans.holmberg@cnexlabs.com>
> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com>
> ---
>   drivers/lightnvm/pblk-recovery.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
> index 009faf5db40f..3fcf062d752c 100644
> --- a/drivers/lightnvm/pblk-recovery.c
> +++ b/drivers/lightnvm/pblk-recovery.c
> @@ -376,12 +376,14 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
>   		rq_ppas = pblk->min_write_pgs;
>   	rq_len = rq_ppas * geo->csecs;
>   
> +retry_rq:
>   	bio = bio_map_kern(dev->q, data, rq_len, GFP_KERNEL);
>   	if (IS_ERR(bio))
>   		return PTR_ERR(bio);
>   
>   	bio->bi_iter.bi_sector = 0; /* internal bio */
>   	bio_set_op_attrs(bio, REQ_OP_READ, 0);
> +	bio_get(bio);
>   
>   	rqd->bio = bio;
>   	rqd->opcode = NVM_OP_PREAD;
> @@ -394,7 +396,6 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
>   	if (pblk_io_aligned(pblk, rq_ppas))
>   		rqd->is_seq = 1;
>   
> -retry_rq:
>   	for (i = 0; i < rqd->nr_ppas; ) {
>   		struct ppa_addr ppa;
>   		int pos;
> @@ -417,6 +418,7 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
>   	if (ret) {
>   		pblk_err(pblk, "I/O submission failed: %d\n", ret);
>   		bio_put(bio);
> +		bio_put(bio);
>   		return ret;
>   	}
>   
> @@ -428,19 +430,25 @@ static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
>   
>   		if (padded) {
>   			pblk_log_read_err(pblk, rqd);
> +			bio_put(bio);
>   			return -EINTR;
>   		}
>   
>   		pad_distance = pblk_pad_distance(pblk, line);
>   		ret = pblk_recov_pad_line(pblk, line, pad_distance);
> -		if (ret)
> +		if (ret) {
> +			bio_put(bio);
>   			return ret;
> +		}
>   
>   		padded = true;
> +		bio_put(bio);
>   		goto retry_rq;
>   	}
>   
>   	pblk_get_packed_meta(pblk, rqd);
> +	bio_put(bio);
> +
>   	for (i = 0; i < rqd->nr_ppas; i++) {
>   		struct pblk_sec_meta *meta = pblk_get_meta(pblk, meta_list, i);
>   		u64 lba = le64_to_cpu(meta->lba);
> 

Thanks Igor. I've merged this patch with your "lightnvm: pblk: support 
packed metadata" patch.
diff mbox series

Patch

diff --git a/drivers/lightnvm/pblk-recovery.c b/drivers/lightnvm/pblk-recovery.c
index 009faf5db40f..3fcf062d752c 100644
--- a/drivers/lightnvm/pblk-recovery.c
+++ b/drivers/lightnvm/pblk-recovery.c
@@ -376,12 +376,14 @@  static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
 		rq_ppas = pblk->min_write_pgs;
 	rq_len = rq_ppas * geo->csecs;
 
+retry_rq:
 	bio = bio_map_kern(dev->q, data, rq_len, GFP_KERNEL);
 	if (IS_ERR(bio))
 		return PTR_ERR(bio);
 
 	bio->bi_iter.bi_sector = 0; /* internal bio */
 	bio_set_op_attrs(bio, REQ_OP_READ, 0);
+	bio_get(bio);
 
 	rqd->bio = bio;
 	rqd->opcode = NVM_OP_PREAD;
@@ -394,7 +396,6 @@  static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
 	if (pblk_io_aligned(pblk, rq_ppas))
 		rqd->is_seq = 1;
 
-retry_rq:
 	for (i = 0; i < rqd->nr_ppas; ) {
 		struct ppa_addr ppa;
 		int pos;
@@ -417,6 +418,7 @@  static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
 	if (ret) {
 		pblk_err(pblk, "I/O submission failed: %d\n", ret);
 		bio_put(bio);
+		bio_put(bio);
 		return ret;
 	}
 
@@ -428,19 +430,25 @@  static int pblk_recov_scan_oob(struct pblk *pblk, struct pblk_line *line,
 
 		if (padded) {
 			pblk_log_read_err(pblk, rqd);
+			bio_put(bio);
 			return -EINTR;
 		}
 
 		pad_distance = pblk_pad_distance(pblk, line);
 		ret = pblk_recov_pad_line(pblk, line, pad_distance);
-		if (ret)
+		if (ret) {
+			bio_put(bio);
 			return ret;
+		}
 
 		padded = true;
+		bio_put(bio);
 		goto retry_rq;
 	}
 
 	pblk_get_packed_meta(pblk, rqd);
+	bio_put(bio);
+
 	for (i = 0; i < rqd->nr_ppas; i++) {
 		struct pblk_sec_meta *meta = pblk_get_meta(pblk, meta_list, i);
 		u64 lba = le64_to_cpu(meta->lba);