Re: [PATCH parted 2/2] linux: Fix use after free in devicemapper code

[Jim Meyering]

Hans de Goede wrote:
>> Is this adjustment ok with you, since I'm leaving your name on it?
>> I don't want to leave the assignments to NULL, since they're
>> effectively dead code and I don't want to provoke warnings
>> from the likes of clang and coverity.
>>
>> I understand the motivation in setting them to NULL,
>> but since there is obviously no further use of dev_name,
>> and the next use of "task" is an assignment, it seems safe.
>>
>
> You cannot remove the "task = NULL" statement, otherwise the task will
> get destroyed a second time in the error path of the asprintf directly
> below the moved "dm_task_destroy (task)"

Oh.  Duh ;-)

I'll fold this into it, also removing a useless initialization.

>From d1f4354cc2bd3ffae2afbb510bafd43f2b6857ba Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 18 Dec 2009 13:51:41 +0100
Subject: [PATCH] .

---
 libparted/arch/linux.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index d996f32..aefe788 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2423,7 +2423,6 @@ err:
 static int
 _dm_add_partition (PedDisk* disk, PedPartition* part)
 {
-        struct dm_task* task = NULL;
         int             rc;
         char*           vol_name = NULL;
         const char*     dev_name = NULL;
@@ -2434,7 +2433,7 @@ _dm_add_partition (PedDisk* disk, PedPartition* part)
                 return 0;

         /* Get map name from devicemapper */
-        task = dm_task_create (DM_DEVICE_INFO);
+        struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
         if (!task)
                 goto err;

@@ -2453,6 +2452,7 @@ _dm_add_partition (PedDisk* disk, PedPartition* part)

         /* Caution: dm_task_destroy frees dev_name.  */
         dm_task_destroy (task);
+        task = NULL;

         if (asprintf (&params, "%d:%d %lld", arch_specific->major,
                       arch_specific->minor, part->geom.start) == -1)
--
1.6.6.rc3.271.g3d40f