linux-kernel - Re: [PATCH] lightnvm: pblk: fix read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Jun 2018 07:37:30 +0000
From:   Javier Gonzalez <javier@...xlabs.com>
To:     Matias Bjørling <mb@...htnvm.io>
CC:     "Konopko, Igor J" <igor.j.konopko@...el.com>,
        "marcin.dziegielewski@...el.com" <marcin.dziegielewski@...el.com>,
        Hans Holmberg <hans.holmberg@...xlabs.com>,
        Heiner Litz <hlitz@...c.edu>,
        Young Tack Tack Jin <youngtack.jin@...cuitblvd.com>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] lightnvm: pblk: fix read_bitmap for 32bit archs

> On 27 Jun 2018, at 21.42, Matias Bjørling <mb@...htnvm.io> wrote:
> 
> If using pblk on a 32bit architecture, and there is a need to
> perform a partial read, the partial read bitmap will only have
> allocated 32 entries, where as 64 are needed.
> 
> Make sure that the read_bitmap is initialized to 64bits on 32bit
> architectures as well.
> 
> Signed-off-by: Matias Bjørling <mb@...htnvm.io>
> ---
> drivers/lightnvm/pblk-read.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/lightnvm/pblk-read.c b/drivers/lightnvm/pblk-read.c
> index 6e93c489ce57..671635275d56 100644
> --- a/drivers/lightnvm/pblk-read.c
> +++ b/drivers/lightnvm/pblk-read.c
> @@ -401,7 +401,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
> 	struct pblk_g_ctx *r_ctx;
> 	struct nvm_rq *rqd;
> 	unsigned int bio_init_idx;
> -	unsigned long read_bitmap; /* Max 64 ppas per request */
> +	DECLARE_BITMAP(read_bitmap, 64); /* Max 64 ppas per request */

When we removed max_hx_sect we added a constant (NVM_MAX_VLBA). Use this
instead? Now I see Igor already commented on this...

> 	int ret = NVM_IO_ERR;
> 
> 	/* logic error: lba out-of-bounds. Ignore read request */
> @@ -413,7 +413,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
> 
> 	generic_start_io_acct(q, READ, bio_sectors(bio), &pblk->disk->part0);
> 
> -	bitmap_zero(&read_bitmap, nr_secs);
> +	bitmap_zero(read_bitmap, nr_secs);
> 
> 	rqd = pblk_alloc_rqd(pblk, PBLK_READ);
> 
> @@ -444,19 +444,19 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
> 		rqd->ppa_list = rqd->meta_list + pblk_dma_meta_size;
> 		rqd->dma_ppa_list = rqd->dma_meta_list + pblk_dma_meta_size;
> 
> -		pblk_read_ppalist_rq(pblk, rqd, bio, blba, &read_bitmap);
> +		pblk_read_ppalist_rq(pblk, rqd, bio, blba, read_bitmap);
> 	} else {
> -		pblk_read_rq(pblk, rqd, bio, blba, &read_bitmap);
> +		pblk_read_rq(pblk, rqd, bio, blba, read_bitmap);
> 	}
> 
> -	if (bitmap_full(&read_bitmap, nr_secs)) {
> +	if (bitmap_full(read_bitmap, nr_secs)) {
> 		atomic_inc(&pblk->inflight_io);
> 		__pblk_end_io_read(pblk, rqd, false);
> 		return NVM_IO_DONE;
> 	}
> 
> 	/* All sectors are to be read from the device */
> -	if (bitmap_empty(&read_bitmap, rqd->nr_ppas)) {
> +	if (bitmap_empty(read_bitmap, rqd->nr_ppas)) {
> 		struct bio *int_bio = NULL;
> 
> 		/* Clone read bio to deal with read errors internally */
> @@ -480,7 +480,7 @@ int pblk_submit_read(struct pblk *pblk, struct bio *bio)
> 	/* The read bio request could be partially filled by the write buffer,
> 	 * but there are some holes that need to be read from the drive.
> 	 */
> -	return pblk_partial_read(pblk, rqd, bio, bio_init_idx, &read_bitmap);
> +	return pblk_partial_read(pblk, rqd, bio, bio_init_idx, read_bitmap);
> 
> fail_rqd_free:
> 	pblk_free_rqd(pblk, rqd, PBLK_READ);
> --
> 2.11.0

Thanks for fixing it.


Reviewed-by: Javier González <javier@...xlabs.com>


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)