All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2/2] ubi: free the normal volumes in error paths of ubi_attach_mtd_dev()
@ 2019-11-30  9:48 Hou Tao
  2019-12-12  9:14 ` Hou Tao
  0 siblings, 1 reply; 3+ messages in thread
From: Hou Tao @ 2019-11-30  9:48 UTC (permalink / raw)
  To: linux-mtd
  Cc: vigneshr, dedekind1, richard, marek.vasut, miquel.raynal,
	computersforpeace, dwmw2

The allocated normal volumes saved in ubi->volumes are not freed
in the error paths in ubi_attach_mtd_dev() and its callees (e.g.
ubi_attach() and ubi_read_volume_table()).

These normal volumes should be freed through kill_volumes() and
vol_release(), but ubi_attach_mtd_dev() may fail before
calling uif_init(), and there will be memory leaks.

So adding a new helper ubi_free_all_volumes() to free the normal
and the internal volumes. And in order to prevent double-free
of volume, reset ubi->volumes[i] to NULL after freeing.

Signed-off-by: Hou Tao <houtao1@huawei.com>
---
 drivers/mtd/ubi/attach.c |  2 +-
 drivers/mtd/ubi/build.c  | 31 ++++++++++++++++++++++++++-----
 drivers/mtd/ubi/ubi.h    |  1 +
 drivers/mtd/ubi/vtbl.c   | 10 ++--------
 4 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
index 10b2459f8951..ea7440ac913b 100644
--- a/drivers/mtd/ubi/attach.c
+++ b/drivers/mtd/ubi/attach.c
@@ -1640,7 +1640,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
 out_wl:
 	ubi_wl_close(ubi);
 out_vtbl:
-	ubi_free_internal_volumes(ubi);
+	ubi_free_all_volumes(ubi);
 	vfree(ubi->vtbl);
 out_ai:
 	destroy_ai(ai);
diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index d636bbe214cb..25fb72b2efa0 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -503,21 +503,42 @@ static void uif_close(struct ubi_device *ubi)
 }
 
 /**
- * ubi_free_internal_volumes - free internal volumes.
+ * ubi_free_volumes_from - free volumes from specific index.
  * @ubi: UBI device description object
+ * @from: the start index used for volume free.
  */
-void ubi_free_internal_volumes(struct ubi_device *ubi)
+static void ubi_free_volumes_from(struct ubi_device *ubi, int from)
 {
 	int i;
 
-	for (i = ubi->vtbl_slots;
-	     i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) {
+	for (i = from; i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) {
+		if (!ubi->volumes[i])
+			continue;
 		ubi_eba_replace_table(ubi->volumes[i], NULL);
 		ubi_fastmap_destroy_checkmap(ubi->volumes[i]);
 		kfree(ubi->volumes[i]);
+		ubi->volumes[i] = NULL;
 	}
 }
 
+/**
+ * ubi_free_all_volumes - free all volumes.
+ * @ubi: UBI device description object
+ */
+void ubi_free_all_volumes(struct ubi_device *ubi)
+{
+	ubi_free_volumes_from(ubi, 0);
+}
+
+/**
+ * ubi_free_internal_volumes - free internal volumes.
+ * @ubi: UBI device description object
+ */
+void ubi_free_internal_volumes(struct ubi_device *ubi)
+{
+	ubi_free_volumes_from(ubi, ubi->vtbl_slots);
+}
+
 static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
 {
 	int limit, device_pebs;
@@ -1013,7 +1034,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
 out_detach:
 	ubi_devices[ubi_num] = NULL;
 	ubi_wl_close(ubi);
-	ubi_free_internal_volumes(ubi);
+	ubi_free_all_volumes(ubi);
 	vfree(ubi->vtbl);
 out_free:
 	vfree(ubi->peb_buf);
diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
index 721b6aa7936c..5ab3affd2fcf 100644
--- a/drivers/mtd/ubi/ubi.h
+++ b/drivers/mtd/ubi/ubi.h
@@ -948,6 +948,7 @@ int ubi_volume_notify(struct ubi_device *ubi, struct ubi_volume *vol,
 int ubi_notify_all(struct ubi_device *ubi, int ntype,
 		   struct notifier_block *nb);
 int ubi_enumerate_volumes(struct notifier_block *nb);
+void ubi_free_all_volumes(struct ubi_device *ubi);
 void ubi_free_internal_volumes(struct ubi_device *ubi);
 
 /* kapi.c */
diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
index 8a2a0f091598..f700f0e4f2ec 100644
--- a/drivers/mtd/ubi/vtbl.c
+++ b/drivers/mtd/ubi/vtbl.c
@@ -782,7 +782,7 @@ static int check_attaching_info(const struct ubi_device *ubi,
  */
 int ubi_read_volume_table(struct ubi_device *ubi, struct ubi_attach_info *ai)
 {
-	int i, err;
+	int err;
 	struct ubi_ainf_volume *av;
 
 	empty_vtbl_record.crc = cpu_to_be32(0xf116c36b);
@@ -851,13 +851,7 @@ int ubi_read_volume_table(struct ubi_device *ubi, struct ubi_attach_info *ai)
 
 out_free:
 	vfree(ubi->vtbl);
-	for (i = 0; i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) {
-		if (!ubi->volumes[i])
-			continue;
-		ubi_fastmap_destroy_checkmap(ubi->volumes[i]);
-		kfree(ubi->volumes[i]);
-		ubi->volumes[i] = NULL;
-	}
+	ubi_free_all_volumes(ubi);
 	return err;
 }
 
-- 
2.22.0


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] ubi: free the normal volumes in error paths of ubi_attach_mtd_dev()
  2019-11-30  9:48 [PATCH 2/2] ubi: free the normal volumes in error paths of ubi_attach_mtd_dev() Hou Tao
@ 2019-12-12  9:14 ` Hou Tao
  2020-01-16 23:01   ` Richard Weinberger
  0 siblings, 1 reply; 3+ messages in thread
From: Hou Tao @ 2019-12-12  9:14 UTC (permalink / raw)
  To: linux-mtd
  Cc: vigneshr, dedekind1, richard, marek.vasut, miquel.raynal,
	computersforpeace, dwmw2

ping ?

On 2019/11/30 17:48, Hou Tao wrote:
> The allocated normal volumes saved in ubi->volumes are not freed
> in the error paths in ubi_attach_mtd_dev() and its callees (e.g.
> ubi_attach() and ubi_read_volume_table()).
> 
> These normal volumes should be freed through kill_volumes() and
> vol_release(), but ubi_attach_mtd_dev() may fail before
> calling uif_init(), and there will be memory leaks.
> 
> So adding a new helper ubi_free_all_volumes() to free the normal
> and the internal volumes. And in order to prevent double-free
> of volume, reset ubi->volumes[i] to NULL after freeing.
> 
> Signed-off-by: Hou Tao <houtao1@huawei.com>
> ---
>  drivers/mtd/ubi/attach.c |  2 +-
>  drivers/mtd/ubi/build.c  | 31 ++++++++++++++++++++++++++-----
>  drivers/mtd/ubi/ubi.h    |  1 +
>  drivers/mtd/ubi/vtbl.c   | 10 ++--------
>  4 files changed, 30 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/attach.c b/drivers/mtd/ubi/attach.c
> index 10b2459f8951..ea7440ac913b 100644
> --- a/drivers/mtd/ubi/attach.c
> +++ b/drivers/mtd/ubi/attach.c
> @@ -1640,7 +1640,7 @@ int ubi_attach(struct ubi_device *ubi, int force_scan)
>  out_wl:
>  	ubi_wl_close(ubi);
>  out_vtbl:
> -	ubi_free_internal_volumes(ubi);
> +	ubi_free_all_volumes(ubi);
>  	vfree(ubi->vtbl);
>  out_ai:
>  	destroy_ai(ai);
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index d636bbe214cb..25fb72b2efa0 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -503,21 +503,42 @@ static void uif_close(struct ubi_device *ubi)
>  }
>  
>  /**
> - * ubi_free_internal_volumes - free internal volumes.
> + * ubi_free_volumes_from - free volumes from specific index.
>   * @ubi: UBI device description object
> + * @from: the start index used for volume free.
>   */
> -void ubi_free_internal_volumes(struct ubi_device *ubi)
> +static void ubi_free_volumes_from(struct ubi_device *ubi, int from)
>  {
>  	int i;
>  
> -	for (i = ubi->vtbl_slots;
> -	     i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) {
> +	for (i = from; i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) {
> +		if (!ubi->volumes[i])
> +			continue;
>  		ubi_eba_replace_table(ubi->volumes[i], NULL);
>  		ubi_fastmap_destroy_checkmap(ubi->volumes[i]);
>  		kfree(ubi->volumes[i]);
> +		ubi->volumes[i] = NULL;
>  	}
>  }
>  
> +/**
> + * ubi_free_all_volumes - free all volumes.
> + * @ubi: UBI device description object
> + */
> +void ubi_free_all_volumes(struct ubi_device *ubi)
> +{
> +	ubi_free_volumes_from(ubi, 0);
> +}
> +
> +/**
> + * ubi_free_internal_volumes - free internal volumes.
> + * @ubi: UBI device description object
> + */
> +void ubi_free_internal_volumes(struct ubi_device *ubi)
> +{
> +	ubi_free_volumes_from(ubi, ubi->vtbl_slots);
> +}
> +
>  static int get_bad_peb_limit(const struct ubi_device *ubi, int max_beb_per1024)
>  {
>  	int limit, device_pebs;
> @@ -1013,7 +1034,7 @@ int ubi_attach_mtd_dev(struct mtd_info *mtd, int ubi_num,
>  out_detach:
>  	ubi_devices[ubi_num] = NULL;
>  	ubi_wl_close(ubi);
> -	ubi_free_internal_volumes(ubi);
> +	ubi_free_all_volumes(ubi);
>  	vfree(ubi->vtbl);
>  out_free:
>  	vfree(ubi->peb_buf);
> diff --git a/drivers/mtd/ubi/ubi.h b/drivers/mtd/ubi/ubi.h
> index 721b6aa7936c..5ab3affd2fcf 100644
> --- a/drivers/mtd/ubi/ubi.h
> +++ b/drivers/mtd/ubi/ubi.h
> @@ -948,6 +948,7 @@ int ubi_volume_notify(struct ubi_device *ubi, struct ubi_volume *vol,
>  int ubi_notify_all(struct ubi_device *ubi, int ntype,
>  		   struct notifier_block *nb);
>  int ubi_enumerate_volumes(struct notifier_block *nb);
> +void ubi_free_all_volumes(struct ubi_device *ubi);
>  void ubi_free_internal_volumes(struct ubi_device *ubi);
>  
>  /* kapi.c */
> diff --git a/drivers/mtd/ubi/vtbl.c b/drivers/mtd/ubi/vtbl.c
> index 8a2a0f091598..f700f0e4f2ec 100644
> --- a/drivers/mtd/ubi/vtbl.c
> +++ b/drivers/mtd/ubi/vtbl.c
> @@ -782,7 +782,7 @@ static int check_attaching_info(const struct ubi_device *ubi,
>   */
>  int ubi_read_volume_table(struct ubi_device *ubi, struct ubi_attach_info *ai)
>  {
> -	int i, err;
> +	int err;
>  	struct ubi_ainf_volume *av;
>  
>  	empty_vtbl_record.crc = cpu_to_be32(0xf116c36b);
> @@ -851,13 +851,7 @@ int ubi_read_volume_table(struct ubi_device *ubi, struct ubi_attach_info *ai)
>  
>  out_free:
>  	vfree(ubi->vtbl);
> -	for (i = 0; i < ubi->vtbl_slots + UBI_INT_VOL_COUNT; i++) {
> -		if (!ubi->volumes[i])
> -			continue;
> -		ubi_fastmap_destroy_checkmap(ubi->volumes[i]);
> -		kfree(ubi->volumes[i]);
> -		ubi->volumes[i] = NULL;
> -	}
> +	ubi_free_all_volumes(ubi);
>  	return err;
>  }
>  
> 


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 2/2] ubi: free the normal volumes in error paths of ubi_attach_mtd_dev()
  2019-12-12  9:14 ` Hou Tao
@ 2020-01-16 23:01   ` Richard Weinberger
  0 siblings, 0 replies; 3+ messages in thread
From: Richard Weinberger @ 2020-01-16 23:01 UTC (permalink / raw)
  To: Hou Tao
  Cc: Vignesh Raghavendra, Artem Bityutskiy, Richard Weinberger,
	Marek Vasut, linux-mtd, Miquel Raynal, Brian Norris,
	David Woodhouse

On Thu, Dec 12, 2019 at 10:14 AM Hou Tao <houtao1@huawei.com> wrote:
>
> ping ?

Applied. Thank you, Hou Tao.

-- 
Thanks,
//richard

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-16 23:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-30  9:48 [PATCH 2/2] ubi: free the normal volumes in error paths of ubi_attach_mtd_dev() Hou Tao
2019-12-12  9:14 ` Hou Tao
2020-01-16 23:01   ` Richard Weinberger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.