* [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list @ 2018-12-07 8:25 Igor Konopko 2018-12-07 8:25 ` [PATCH v2 2/2] lightnvm: pblk: Ensure that bio is not freed on recovery Igor Konopko 2018-12-07 12:00 ` [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list Matias Bjørling 0 siblings, 2 replies; 4+ messages in thread From: Igor Konopko @ 2018-12-07 8:25 UTC (permalink / raw) To: mb; +Cc: linux-block, javier, hans.holmberg, igor.j.konopko Currently when using PBLK with 0 sized metadata both ppa list and meta list points to the same memory since pblk_dma_meta_size() returns 0 in that case. This commit fix that issue by ensuring that pblk_dma_meta_size() always returns space equal to sizeof(struct pblk_sec_meta) and thus ppa list and meta list points to different memory address. Even that in that case drive does not really care about meta_list pointer, this is the easiest way to fix that issue without introducing changes in many places in the code just for 0 sized metadata case. The same approach needs to be also done for pblk_get_sec_meta() since we also cannot point to the same memory address in meta buffer when we are using it for pblk recovery process Reported-by: Hans Holmberg <hans.holmberg@cnexlabs.com> Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> --- drivers/lightnvm/pblk.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h index bc40b1381ff6..85e38ed62f85 100644 --- a/drivers/lightnvm/pblk.h +++ b/drivers/lightnvm/pblk.h @@ -1388,12 +1388,15 @@ static inline unsigned int pblk_get_min_chks(struct pblk *pblk) static inline struct pblk_sec_meta *pblk_get_meta(struct pblk *pblk, void *meta, int index) { - return meta + pblk->oob_meta_size * index; + return meta + + max_t(int, sizeof(struct pblk_sec_meta), pblk->oob_meta_size) + * index; } static inline int pblk_dma_meta_size(struct pblk *pblk) { - return pblk->oob_meta_size * NVM_MAX_VLBA; + return max_t(int, sizeof(struct pblk_sec_meta), pblk->oob_meta_size) + * NVM_MAX_VLBA; } static inline int pblk_is_oob_meta_supported(struct pblk *pblk) -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] lightnvm: pblk: Ensure that bio is not freed on recovery 2018-12-07 8:25 [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list Igor Konopko @ 2018-12-07 8:25 ` Igor Konopko 2018-12-07 12:01 ` Matias Bjørling 2018-12-07 12:00 ` [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list Matias Bjørling 1 sibling, 1 reply; 4+ messages in thread From: Igor Konopko @ 2018-12-07 8:25 UTC (permalink / raw) To: mb; +Cc: linux-block, javier, hans.holmberg, igor.j.konopko 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); -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] lightnvm: pblk: Ensure that bio is not freed on recovery 2018-12-07 8:25 ` [PATCH v2 2/2] lightnvm: pblk: Ensure that bio is not freed on recovery Igor Konopko @ 2018-12-07 12:01 ` Matias Bjørling 0 siblings, 0 replies; 4+ messages in thread From: Matias Bjørling @ 2018-12-07 12:01 UTC (permalink / raw) To: Igor Konopko; +Cc: linux-block, javier, hans.holmberg 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list 2018-12-07 8:25 [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list Igor Konopko 2018-12-07 8:25 ` [PATCH v2 2/2] lightnvm: pblk: Ensure that bio is not freed on recovery Igor Konopko @ 2018-12-07 12:00 ` Matias Bjørling 1 sibling, 0 replies; 4+ messages in thread From: Matias Bjørling @ 2018-12-07 12:00 UTC (permalink / raw) To: Igor Konopko; +Cc: linux-block, javier, hans.holmberg On 12/07/2018 09:25 AM, Igor Konopko wrote: > Currently when using PBLK with 0 sized metadata both ppa list > and meta list points to the same memory since pblk_dma_meta_size() > returns 0 in that case. > > This commit fix that issue by ensuring that pblk_dma_meta_size() > always returns space equal to sizeof(struct pblk_sec_meta) and thus > ppa list and meta list points to different memory address. > > Even that in that case drive does not really care about meta_list > pointer, this is the easiest way to fix that issue without introducing > changes in many places in the code just for 0 sized metadata case. > > The same approach needs to be also done for pblk_get_sec_meta() > since we also cannot point to the same memory address in meta buffer > when we are using it for pblk recovery process > > Reported-by: Hans Holmberg <hans.holmberg@cnexlabs.com> > Signed-off-by: Igor Konopko <igor.j.konopko@intel.com> > --- > drivers/lightnvm/pblk.h | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h > index bc40b1381ff6..85e38ed62f85 100644 > --- a/drivers/lightnvm/pblk.h > +++ b/drivers/lightnvm/pblk.h > @@ -1388,12 +1388,15 @@ static inline unsigned int pblk_get_min_chks(struct pblk *pblk) > static inline struct pblk_sec_meta *pblk_get_meta(struct pblk *pblk, > void *meta, int index) > { > - return meta + pblk->oob_meta_size * index; > + return meta + > + max_t(int, sizeof(struct pblk_sec_meta), pblk->oob_meta_size) > + * index; > } > > static inline int pblk_dma_meta_size(struct pblk *pblk) > { > - return pblk->oob_meta_size * NVM_MAX_VLBA; > + return max_t(int, sizeof(struct pblk_sec_meta), pblk->oob_meta_size) > + * NVM_MAX_VLBA; > } > > static inline int pblk_is_oob_meta_supported(struct pblk *pblk) > Thanks Igor. It's applied for 4.21. I've updated the description a bit ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-12-07 12:01 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-07 8:25 [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list Igor Konopko 2018-12-07 8:25 ` [PATCH v2 2/2] lightnvm: pblk: Ensure that bio is not freed on recovery Igor Konopko 2018-12-07 12:01 ` Matias Bjørling 2018-12-07 12:00 ` [PATCH v2 1/2] lightnvm: pblk: Do not overwrite ppa list with meta list Matias Bjørling
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).