cciss: memory leak in cciss_init_one()

PATCH New Eric Dumazet 3 participants 5 messages Aug 5, 2009 – Aug 24, 2009 Block layer

Share & tools

Eric Dumazet

author

2009-08-05 10:42 UTC #0 lore raw

PATCH 1/1 cciss: memory leak in cciss_init_one()

commit 22bece00dc1f28dd3374c55e464c9f02eb642876 (cciss: fix regression firmware not displayed in procfs) added a small memory leak in cciss_init_one()

Signed-off-by Eric Dumazet <[email protected]>

---

drivers/block/cciss.c +2 -2

index a52cc7f..0589dfb 100644
@@ -3889,7 +3889,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 	int j = 0;
 	int rc;
 	int dac, return_code;
-	InquiryData_struct *inq_buff = NULL;
+	InquiryData_struct *inq_buff;
 
 	if (reset_devices) {
 		/* Reset the controller with a PCI power-cycle */

@@ -4029,6 +4029,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 		printk(KERN_WARNING "cciss: unable to determine firmware"
 			" version of controller\n");
 	}
+	kfree(inq_buff);
 
 	cciss_procinit(i);

@@ -4045,7 +4046,6 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 	return 1;
 
 clean4:
-	kfree(inq_buff);
 	kfree(hba[i]->cmd_pool_bits);
 	if (hba[i]->cmd_pool)
 		pci_free_consistent(hba[i]->pdev,

Eric Dumazet

author

2009-08-23 09:03 UTC #1 lore raw

PATCH 1/1 cciss: memory leak in cciss_init_one()

Andrew, I originally sent this 18 days ago and got no reply yet, maybe could you get the baby ?

Thanks

[PATCH] cciss: memory leak in cciss_init_one()

commit 22bece00dc1f28dd3374c55e464c9f02eb642876 (cciss: fix regression firmware not displayed in procfs) added a small memory leak in cciss_init_one()

Signed-off-by Eric Dumazet <[email protected]>

---

drivers/block/cciss.c +2 -2

index a52cc7f..0589dfb 100644
@@ -3889,7 +3889,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 	int j = 0;
 	int rc;
 	int dac, return_code;
-	InquiryData_struct *inq_buff = NULL;
+	InquiryData_struct *inq_buff;
 
 	if (reset_devices) {
 		/* Reset the controller with a PCI power-cycle */

@@ -4029,6 +4029,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 		printk(KERN_WARNING "cciss: unable to determine firmware"
 			" version of controller\n");
 	}
+	kfree(inq_buff);
 
 	cciss_procinit(i);

@@ -4045,7 +4046,6 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 	return 1;
 
 clean4:
-	kfree(inq_buff);
 	kfree(hba[i]->cmd_pool_bits);
 	if (hba[i]->cmd_pool)
 		pci_free_consistent(hba[i]->pdev,

Jens Axboe

2009-08-24 08:01 UTC #2 lore raw

Quoted 2 lines · Eric Dumazet

Andrew, I originally sent this 18 days ago and got no reply yet, maybe could you get the baby ?

I'll add it for 2.6.32.

Quoted 41 lines

Thanks

[PATCH] cciss: memory leak in cciss_init_one()

commit 22bece00dc1f28dd3374c55e464c9f02eb642876 (cciss: fix regression firmware not displayed in procfs) added a small memory leak in cciss_init_one()

Signed-off-by Eric Dumazet <[email protected]>

---

drivers/block/cciss.c +2 -2

index a52cc7f..0589dfb 100644
@@ -3889,7 +3889,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 	int j = 0;
 	int rc;
 	int dac, return_code;
-	InquiryData_struct *inq_buff = NULL;
+	InquiryData_struct *inq_buff;
 
 	if (reset_devices) {
 		/* Reset the controller with a PCI power-cycle */

@@ -4029,6 +4029,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 		printk(KERN_WARNING "cciss: unable to determine firmware"
 			" version of controller\n");
 	}
+	kfree(inq_buff);
 
 	cciss_procinit(i);

@@ -4045,7 +4046,6 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
 	return 1;
 
 clean4:
-	kfree(inq_buff);
 	kfree(hba[i]->cmd_pool_bits);
 	if (hba[i]->cmd_pool)
 		pci_free_consistent(hba[i]->pdev,

Signature · 1 line

Jens Axboe

Miller, Mike (OS Dev)

2009-08-24 16:04 UTC #3 lore raw

Quoted 10 lines

-----Original Message-----
From: Jens Axboe [mailto:[email protected]]
Sent: Monday, August 24, 2009 3:01 AM
To: Eric Dumazet
Cc: Andrew Morton; Miller, Mike (OS Dev); linux kernel; [email protected]; FUJITA Tomonori
Subject: Re: [PATCH] cciss: memory leak in cciss_init_one()

Quoted 7 lines

Quoted 1 line · Eric Dumazet
Andrew, I originally sent this 18 days ago and got no reply
yet, maybe
Quoted 1 line
could you get the baby ?
I'll add it for 2.6.32.

I guess I'm missing something. Where is the leak?

-- mikem

Quoted 51 lines

Quoted 7 lines
Thanks
[PATCH] cciss: memory leak in cciss_init_one()
commit 22bece00dc1f28dd3374c55e464c9f02eb642876 (cciss: fix regression firmware not displayed in procfs)
added a small
drivers/block/cciss.c static int __devinit
Quoted 9 lines
memory leak in cciss_init_one()
Signed-off-by Eric Dumazet <[email protected]>
---
drivers/block/cciss.c index renamed from drivers/block/cciss.c +0 -0
a52cc7f..0589dfb 100644
drivers/block/cciss.c +0 -0
@@ -3889,7 +3889,7 @@ static int __devinit 
cciss_init_one(struct pci_dev *pdev,
drivers/block/cciss.c static int __devinit cciss_init_one(struct pci_dev *pdev,
Quoted 8 lines
drivers/block/cciss.c +1 -1
 	int j = 0;
 	int rc;
 	int dac, return_code;
-	InquiryData_struct *inq_buff = NULL;
+	InquiryData_struct *inq_buff;
 
 	if (reset_devices) {
 		/* Reset the controller with a PCI power-cycle 
*/ @@ -4029,6
Quoted 2 lines
+4029,7 @@ static int __devinit cciss_init_one(struct pci_dev *pdev,
printk(KERN_WARNING "cciss: unable to determine
firmware"
drivers/block/cciss.c static int __devinit
Quoted 7 lines
drivers/block/cciss.c +1 -0
 			" version of controller\n");
 	}
+	kfree(inq_buff);
 
 	cciss_procinit(i);
@@ -4045,7 +4046,6 @@ static int __devinit 
cciss_init_one(struct pci_dev *pdev,
drivers/block/cciss.c static int __devinit cciss_init_one(struct pci_dev *pdev, · line 4051
Quoted 8 lines
drivers/block/cciss.c +0 -1
 	return 1;
 
 clean4:
-	kfree(inq_buff);
 	kfree(hba[i]->cmd_pool_bits);
 	if (hba[i]->cmd_pool)
 		pci_free_consistent(hba[i]->pdev,
--
Jens Axboe

Eric Dumazet

author

2009-08-24 16:38 UTC #4 lore raw

Quoted 10 lines · Mike (OS Dev)

Quoted 10 lines
-----Original Message-----
From: Jens Axboe [mailto:[email protected]]
Sent: Monday, August 24, 2009 3:01 AM
To: Eric Dumazet
Cc: Andrew Morton; Miller, Mike (OS Dev); linux kernel; [email protected]; FUJITA Tomonori
Subject: Re: [PATCH] cciss: memory leak in cciss_init_one()

Quoted 8 lines

Quoted 6 lines · Eric Dumazet
Quoted 1 line
Andrew, I originally sent this 18 days ago and got no reply
yet, maybe
Quoted 1 line
could you get the baby ?
I'll add it for 2.6.32.
I guess I'm missing something. Where is the leak?

Well...

Problem is : In normal path, kfree(inq_buff); is not called.

It is called only if a "goto clean4;" is done (error path, and in this case, inq_buff is NULL anyway)

So we exit from cciss_init_one() without freeing
a "InquiryData_struct" structure.

I am pretty sure kmemleak would complain on this leak :)