Reviewed-By : Vladimir Serbinenko<phco...@gmail.com>
Le jeu. 27 mars 2025, 20:57, Lidong Chen via Grub-devel <grub-devel@gnu.org>
a écrit :
> Fix memory leaks in make_vg() with new helper functions, free_pv()
> and free_lv(). Additionally, correct a check after allocating
> comp->segments->nodes that mistakenly checked lv->segments->nodes
> instead, likely due to a copy-paste error.
>
> Fixes: CID 473878
> Fixes: CID 473884
> Fixes: CID 473889
> Fixes: CID 473890
>
> Signed-off-by: Lidong Chen <lidong.c...@oracle.com>
> ---
> grub-core/disk/ldm.c | 180 +++++++++++++++++++------------------------
> 1 file changed, 80 insertions(+), 100 deletions(-)
>
> diff --git a/grub-core/disk/ldm.c b/grub-core/disk/ldm.c
> index 048e29cd0..0f4f22aaa 100644
> --- a/grub-core/disk/ldm.c
> +++ b/grub-core/disk/ldm.c
> @@ -179,6 +179,36 @@ gpt_ldm_sector (grub_disk_t dsk)
> return sector;
> }
>
> +static void
> +free_pv (struct grub_diskfilter_pv *pv)
> +{
> + if (pv == NULL)
> + return;
> +
> + grub_free (pv->internal_id);
> + grub_free (pv->id.uuid);
> + grub_free (pv);
> +}
> +
> +static void
> +free_lv (struct grub_diskfilter_lv *lv)
> +{
> + if (lv == NULL)
> + return;
> +
> + grub_free (lv->internal_id);
> + grub_free (lv->name);
> + grub_free (lv->fullname);
> + if (lv->segments)
> + {
> + unsigned int i;
> + for (i = 0; i < lv->segment_count; i++)
> + grub_free (lv->segments[i].nodes);
> + grub_free (lv->segments);
> + }
> + grub_free (lv);
> +}
> +
> static struct grub_diskfilter_vg *
> make_vg (grub_disk_t disk,
> const struct grub_ldm_label *label)
> @@ -196,12 +226,8 @@ make_vg (grub_disk_t disk,
> vg->name = grub_malloc (LDM_NAME_STRLEN + 1);
> vg->uuid = grub_malloc (LDM_GUID_STRLEN + 1);
> if (! vg->uuid || !vg->name)
> - {
> - grub_free (vg->uuid);
> - grub_free (vg->name);
> - grub_free (vg);
> - return NULL;
> - }
> + goto fail1;
> +
> grub_memcpy (vg->uuid, label->group_guid, LDM_GUID_STRLEN);
> grub_memcpy (vg->name, label->group_name, LDM_NAME_STRLEN);
> vg->name[LDM_NAME_STRLEN] = 0;
> @@ -261,7 +287,7 @@ make_vg (grub_disk_t disk,
> pv->internal_id = grub_malloc (sz);
> if (!pv->internal_id)
> {
> - grub_free (pv);
> + free_pv (pv);
> goto fail2;
> }
> grub_memcpy (pv->internal_id, ptr, (grub_size_t) ptr[0] + 1);
> @@ -271,7 +297,7 @@ make_vg (grub_disk_t disk,
> if (ptr + *ptr + 1 >= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (pv);
> + free_pv (pv);
> goto fail2;
> }
> /* ptr = name. */
> @@ -279,23 +305,21 @@ make_vg (grub_disk_t disk,
> if (ptr + *ptr + 1
> >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (pv);
> + free_pv (pv);
> goto fail2;
> }
> pv->id.uuidlen = *ptr;
>
> if (grub_add (pv->id.uuidlen, 1, &sz))
> {
> - grub_free (pv->internal_id);
> - grub_free (pv);
> + free_pv (pv);
> goto fail2;
> }
>
> pv->id.uuid = grub_malloc (sz);
> if (pv->id.uuid == NULL)
> {
> - grub_free (pv->internal_id);
> - grub_free (pv);
> + free_pv (pv);
> goto fail2;
> }
> grub_memcpy (pv->id.uuid, ptr + 1, pv->id.uuidlen);
> @@ -343,7 +367,7 @@ make_vg (grub_disk_t disk,
> lv->segments = grub_zalloc (sizeof (*lv->segments));
> if (!lv->segments)
> {
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> lv->segments->start_extent = 0;
> @@ -354,26 +378,25 @@ make_vg (grub_disk_t disk,
> sizeof
> (*lv->segments->nodes));
> if (!lv->segments->nodes)
> {
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> ptr = vblk[i].dynamic;
> if (ptr + *ptr + 1 >= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> if (grub_add (ptr[0], 2, &sz))
> {
> - grub_free (lv->segments);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> lv->internal_id = grub_malloc (sz);
> if (!lv->internal_id)
> {
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> grub_memcpy (lv->internal_id, ptr, ptr[0] + 1);
> @@ -383,20 +406,18 @@ make_vg (grub_disk_t disk,
> if (ptr + *ptr + 1 >= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> if (grub_add (*ptr, 1, &sz))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> lv->name = grub_malloc (sz);
> if (!lv->name)
> {
> - grub_free (lv->internal_id);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> grub_memcpy (lv->name, ptr + 1, *ptr);
> @@ -405,36 +426,28 @@ make_vg (grub_disk_t disk,
> vg->uuid, lv->name);
> if (!lv->fullname)
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> ptr += *ptr + 1;
> if (ptr + *ptr + 1
> >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> /* ptr = volume type. */
> ptr += *ptr + 1;
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> /* ptr = flags. */
> ptr += *ptr + 1;
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
>
> @@ -443,17 +456,13 @@ make_vg (grub_disk_t disk,
> /* ptr = number of children. */
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> ptr += *ptr + 1;
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
>
> @@ -463,9 +472,7 @@ make_vg (grub_disk_t disk,
> || ptr + *ptr + 1>= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv);
> + free_lv (lv);
> goto fail2;
> }
> lv->size = read_int (ptr + 1, *ptr);
> @@ -515,18 +522,18 @@ make_vg (grub_disk_t disk,
> ptr = vblk[i].dynamic;
> if (ptr + *ptr + 1 >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> if (grub_add (ptr[0], 2, &sz))
> {
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> comp->internal_id = grub_malloc (sz);
> if (!comp->internal_id)
> {
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> grub_memcpy (comp->internal_id, ptr, ptr[0] + 1);
> @@ -535,16 +542,14 @@ make_vg (grub_disk_t disk,
> ptr += *ptr + 1;
> if (ptr + *ptr + 1 >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> /* ptr = name. */
> ptr += *ptr + 1;
> if (ptr + *ptr + 1 >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> /* ptr = state. */
> @@ -554,8 +559,7 @@ make_vg (grub_disk_t disk,
> ptr += 4;
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
>
> @@ -563,16 +567,14 @@ make_vg (grub_disk_t disk,
> ptr += *ptr + 1;
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> ptr += 8 + 8;
> if (ptr + *ptr + 1 >= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> for (lv = vg->lvs; lv; lv = lv->next)
> @@ -583,8 +585,7 @@ make_vg (grub_disk_t disk,
> }
> if (!lv)
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> continue;
> }
> comp->size = lv->size;
> @@ -596,8 +597,7 @@ make_vg (grub_disk_t disk,
> sizeof (*comp->segments));
> if (!comp->segments)
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> }
> @@ -608,8 +608,7 @@ make_vg (grub_disk_t disk,
> comp->segments = grub_malloc (sizeof (*comp->segments));
> if (!comp->segments)
> {
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> comp->segments->start_extent = 0;
> @@ -624,27 +623,21 @@ make_vg (grub_disk_t disk,
> }
> else
> {
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> ptr += *ptr + 1;
> ptr++;
> if (!(vblk[i].flags & 0x10))
> {
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> if (ptr >= vblk[i].dynamic + sizeof (vblk[i].dynamic)
> || ptr + *ptr + 1 >= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> comp->segments->stripe_size = read_int (ptr + 1, *ptr);
> @@ -652,20 +645,16 @@ make_vg (grub_disk_t disk,
> if (ptr + *ptr + 1 >= vblk[i].dynamic
> + sizeof (vblk[i].dynamic))
> {
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> comp->segments->node_count = read_int (ptr + 1, *ptr);
> comp->segments->node_alloc = comp->segments->node_count;
> comp->segments->nodes = grub_calloc
> (comp->segments->node_alloc,
> sizeof
> (*comp->segments->nodes));
> - if (!lv->segments->nodes)
> + if (comp->segments->nodes == NULL)
> {
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> }
> @@ -677,20 +666,14 @@ make_vg (grub_disk_t disk,
> if (grub_mul (lv->segments->node_alloc, 2,
> &lv->segments->node_alloc) ||
> grub_mul (lv->segments->node_alloc, sizeof
> (*lv->segments->nodes), &sz))
> {
> - grub_free (comp->segments->nodes);
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
>
> t = grub_realloc (lv->segments->nodes, sz);
> if (!t)
> {
> - grub_free (comp->segments->nodes);
> - grub_free (comp->segments);
> - grub_free (comp->internal_id);
> - grub_free (comp);
> + free_lv (comp);
> goto fail2;
> }
> lv->segments->nodes = t;
> @@ -830,7 +813,10 @@ make_vg (grub_disk_t disk,
> comp->segments[comp->segment_count].nodes
> = grub_malloc (sizeof
> (*comp->segments[comp->segment_count].nodes));
> if (!comp->segments[comp->segment_count].nodes)
> - goto fail2;
> + {
> + grub_free (comp->segments);
> + goto fail2;
> + }
> comp->segments[comp->segment_count].nodes[0] = part;
> comp->segment_count++;
> }
> @@ -845,25 +831,19 @@ make_vg (grub_disk_t disk,
> struct grub_diskfilter_pv *pv, *next_pv;
> for (lv = vg->lvs; lv; lv = next_lv)
> {
> - unsigned i;
> - for (i = 0; i < lv->segment_count; i++)
> - grub_free (lv->segments[i].nodes);
> -
> next_lv = lv->next;
> - grub_free (lv->segments);
> - grub_free (lv->internal_id);
> - grub_free (lv->name);
> - grub_free (lv->fullname);
> - grub_free (lv);
> + free_lv (lv);
> +
> }
> for (pv = vg->pvs; pv; pv = next_pv)
> {
> next_pv = pv->next;
> - grub_free (pv->id.uuid);
> - grub_free (pv);
> + free_pv (pv);
> }
> }
> + fail1:
> grub_free (vg->uuid);
> + grub_free (vg->name);
> grub_free (vg);
> return NULL;
> }
> --
> 2.34.1
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
