Re: [PATCH 5/5] lightnvm: avoid memory leak when lun_map kcalloc fails

[Johannes Thumshirn]

On 2016-04-05 16:55, Matias BjÃrling wrote:
> A memory leak occurs if the lower page table is initialized and the
> following dev->lun_map fails on allocation.
>
> Rearrange the initialization of lower page table to allow dev->lun_map
> to fail gracefully without memory leak.
>
> Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> Signed-off-by: Matias BjÃrling <m@xxxxxxxxxxx>
> ---
>  drivers/lightnvm/core.c | 51 
> ++++++++++++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c
> index 4cadbe0..fd5a9f3 100644
> --- a/drivers/lightnvm/core.c
> +++ b/drivers/lightnvm/core.c
> @@ -504,6 +504,7 @@ static int nvm_core_init(struct nvm_dev *dev)
>  {
>  	struct nvm_id *id = &dev->identity;
>  	struct nvm_id_group *grp = &id->groups[0];
> +	int ret;
>
>  	/* device values */
>  	dev->nr_chnls = grp->num_ch;
> @@ -522,33 +523,16 @@ static int nvm_core_init(struct nvm_dev *dev)
>  	dev->plane_mode = NVM_PLANE_SINGLE;
>  	dev->max_rq_size = dev->ops->max_phys_sect * dev->sec_size;
>
> -	if (grp->mtype != 0) {
> -		pr_err("nvm: memory type not supported\n");
> -		return -EINVAL;
> -	}
> -
> -	switch (grp->fmtype) {
> -	case NVM_ID_FMTYPE_SLC:
> -		if (nvm_init_slc_tbl(dev, grp))
> -			return -ENOMEM;
> -		break;
> -	case NVM_ID_FMTYPE_MLC:
> -		if (nvm_init_mlc_tbl(dev, grp))
> -			return -ENOMEM;
> -		break;
> -	default:
> -		pr_err("nvm: flash type not supported\n");
> -		return -EINVAL;
> -	}
> -
> -	if (!dev->lps_per_blk)
> -		pr_info("nvm: lower page programming table missing\n");
> -
>  	if (grp->mpos & 0x020202)
>  		dev->plane_mode = NVM_PLANE_DOUBLE;
>  	if (grp->mpos & 0x040404)
>  		dev->plane_mode = NVM_PLANE_QUAD;
>
> +	if (grp->mtype != 0) {
> +		pr_err("nvm: memory type not supported\n");
> +		return -EINVAL;
> +	}
> +
>  	/* calculated values */
>  	dev->sec_per_pl = dev->sec_per_pg * dev->nr_planes;
>  	dev->sec_per_blk = dev->sec_per_pl * dev->pgs_per_blk;
> @@ -560,11 +544,34 @@ static int nvm_core_init(struct nvm_dev *dev)
>  					sizeof(unsigned long), GFP_KERNEL);
>  	if (!dev->lun_map)
>  		return -ENOMEM;
> +
> +	switch (grp->fmtype) {
> +	case NVM_ID_FMTYPE_SLC:
> +		if (nvm_init_slc_tbl(dev, grp)) {
> +			ret = -ENOMEM;
> +			goto err_fmtype;
> +		}
> +		break;
> +	case NVM_ID_FMTYPE_MLC:
> +		if (nvm_init_mlc_tbl(dev, grp)) {
> +			ret = -ENOMEM;
> +			goto err_fmtype;
> +		}
> +		break;
> +	default:
> +		pr_err("nvm: flash type not supported\n");
> +		ret = -EINVAL;
> +		goto err_fmtype;
> +	}
> +
>  	INIT_LIST_HEAD(&dev->online_targets);
>  	mutex_init(&dev->mlock);
>  	spin_lock_init(&dev->lock);
>
>  	return 0;
> +err_fmtype:
> +	kfree(dev->lun_map);
> +	return ret;
>  }
>
>  static void nvm_free(struct nvm_dev *dev)

Reviewed-by: Johannes Thumshirn <jthumshirn@xxxxxxx>