Dan Williams <[email protected]> writes:

> There are a few scenarios where init_active_labels() can return without
> registering deactivate_labels() to run when the region is disabled. In
> particular label error injection creates scenarios where a DIMM is
> disabled, but labels on other DIMMs in the region become activated.
>
> Arrange for init_active_labels() to always register deactivate_labels().
>
> Reported-by: Krzysztof Kensicki <[email protected]>
> Fixes: bf9bccc14c05 ("libnvdimm: pmem label sets and namespace 
> instantiation.")
> Signed-off-by: Dan Williams <[email protected]>

Reviewed-by: Jeff Moyer <[email protected]>


> ---
>  drivers/nvdimm/namespace_devs.c |   17 +++++++++++------
>  1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
> index 2403b71b601e..745478213ff2 100644
> --- a/drivers/nvdimm/namespace_devs.c
> +++ b/drivers/nvdimm/namespace_devs.c
> @@ -2527,7 +2527,7 @@ static void deactivate_labels(void *region)
>  
>  static int init_active_labels(struct nd_region *nd_region)
>  {
> -     int i;
> +     int i, rc = 0;
>  
>       for (i = 0; i < nd_region->ndr_mappings; i++) {
>               struct nd_mapping *nd_mapping = &nd_region->mapping[i];
> @@ -2546,13 +2546,14 @@ static int init_active_labels(struct nd_region 
> *nd_region)
>                       else if (test_bit(NDD_LABELING, &nvdimm->flags))
>                               /* fail, labels needed to disambiguate dpa */;
>                       else
> -                             return 0;
> +                             continue;
>  
>                       dev_err(&nd_region->dev, "%s: is %s, failing probe\n",
>                                       dev_name(&nd_mapping->nvdimm->dev),
>                                       test_bit(NDD_LOCKED, &nvdimm->flags)
>                                       ? "locked" : "disabled");
> -                     return -ENXIO;
> +                     rc = -ENXIO;
> +                     goto out;
>               }
>               nd_mapping->ndd = ndd;
>               atomic_inc(&nvdimm->busy);
> @@ -2586,13 +2587,17 @@ static int init_active_labels(struct nd_region 
> *nd_region)
>                       break;
>       }
>  
> -     if (i < nd_region->ndr_mappings) {
> +     if (i < nd_region->ndr_mappings)
> +             rc = -ENOMEM;
> +
> +out:
> +     if (rc) {
>               deactivate_labels(nd_region);
> -             return -ENOMEM;
> +             return rc;
>       }
>  
>       return devm_add_action_or_reset(&nd_region->dev, deactivate_labels,
> -                     nd_region);
> +                                     nd_region);
>  }
>  
>  int nd_region_register_namespaces(struct nd_region *nd_region, int *err)


Reply via email to