Skip to contentSkip to conversation

pfm_smpl_buffer_alloc needs to free vma on error

PATCH In review Kingsley Cheung 2 participants 2 messages Aug 30, 2002

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 pfm_smpl_buffer_alloc needs to free vma on error

Hi David,

In 2.4.19, arch/ia64/kernel/perfmon.c, I noticed that pfm_smpl_buffer_alloc doesn't seem to be freeing the vma structure it allocates if there's an error. Here's a trivial patch to fix it:

ia64/kernel/perfmon.c 29 Aug 2002 23:29:06 -0000 renamed from ia64/kernel/perfmon.c 5 Jul 2002 06:17:29 -0000 1.1.7.1 +7 -5
@@ -647,15 +647,14 @@
        psb = kmalloc(sizeof(*psb), GFP_KERNEL);
        if (psb = NULL) {
                DBprintk(("Can't allocate sampling buffer descriptor\n"));
-               pfm_rvfree(smpl_buf, size);
-               return -ENOMEM;
+               goto kmalloc_fail;
        }
 
        /* allocate vma */
        vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
        if (!vma) {
                DBprintk(("Cannot allocate vma\n"));
-               goto error;
+               goto kmem_cache_alloc_fail;
        }
        /*
         * partially initialize the vma for the sampling buffer
@@ -750,9 +749,12 @@
 
        return 0;
 
-error:
-       pfm_rvfree(smpl_buf, size);
+ error:
+       kmem_cache_free(vm_area_cachep, vma);
+ kmem_cache_alloc_fail:
        kfree(psb);
+ kmalloc_fail:
+       pfm_rvfree(smpl_buf, size);
        return -ENOMEM;
 }
 

The patch compiles but it hasn't been tested.

Cheers,
Kingsley

#1 lore raw
Quoted 6 lines · Kingsley Cheung

Hi David,

In 2.4.19, arch/ia64/kernel/perfmon.c, I noticed that pfm_smpl_buffer_alloc doesn't seem to be freeing the vma structure it allocates if there's an error. Here's a trivial patch to fix it:

Yes, you are right. I have generated a similar patch with the labels slightly renamed. Note that the patch also fixes another problem with read_unlock() in the same function.

Thanks for spotting this.

--
-Stephane

arch/ia64/kernel/perfmon.c renamed from arch/ia64/kernel/perfmon.c.old +10 -5
@@ -767,15 +767,14 @@
 	psb = kmalloc(sizeof(*psb), GFP_KERNEL);
 	if (psb == NULL) {
 		DBprintk(("Can't allocate sampling buffer descriptor\n"));
-		pfm_rvfree(smpl_buf, size);
-		return -ENOMEM;
+		goto error_kmalloc;
 	}
 
 	/* allocate vma */
 	vma = kmem_cache_alloc(vm_area_cachep, SLAB_KERNEL);
 	if (!vma) {
 		DBprintk(("Cannot allocate vma\n"));
-		goto error;
+		goto error_kmem;
 	}
 	/*
 	 * partially initialize the vma for the sampling buffer
@@ -875,8 +874,11 @@
 	return 0;
 
 error:
-	pfm_rvfree(smpl_buf, size);
+	kmem_cache_free(vm_area_cachep, vma);
+error_kmem:
 	kfree(psb);
+error_kmalloc:
+	pfm_rvfree(smpl_buf, size);
 	return -ENOMEM;
 }
 
@@ -1057,7 +1059,10 @@
 			/*
 			 * check if we can send this task a signal
 			 */
-			if (pfm_bad_permissions(notify_task)) goto buffer_error;
+			if (pfm_bad_permissions(notify_task)) {
+				read_unlock(&tasklist_lock);
+				goto buffer_error;
+			}
 
 			/* 
 		 	 * make visible