aboutsummaryrefslogtreecommitdiffstats
path: root/drivers/block/DAC960.c
diff options
context:
space:
mode:
authorJesper Juhl <jesper.juhl@gmail.com>2006-05-13 18:39:38 -0400
committerJames Bottomley <jejb@mulgrave.il.steeleye.com>2006-05-20 10:23:23 -0400
commit07fb75a50600c0c480b40c6d11dbc993f21bc2bc (patch)
tree9012c689500d33f576c99f00235f44901c86a6cc /drivers/block/DAC960.c
parent5b9851b5511833a96fea2196971b5709ef019136 (diff)
[SCSI] fix (unlikely) memory leak in DAC960 driver
The Coverity checker found a memory leak (bug nr. 1245) in drivers/block/DAC960.c::DAC960_V2_ProcessCompletedCommand() The leak is pretty unlikely since it requires that the first of two successive kmalloc() calls fail while the second one succeeds. But it can still happen even if it's unlikely. If the first call that allocates 'PhysicalDeviceInfo' fails but the one that allocates 'InquiryUnitSerialNumber' succeeds, then we will leak the memory allocated to 'InquiryUnitSerialNumber' when the variable goes out of scope. A simple fix for this is to change the existing code that frees 'PhysicalDeviceInfo' if that one was allocated but 'InquiryUnitSerialNumber' was not, into a check for either pointer being NULL and if so just free both. This is safe since kfree() can deal with being passed a NULL pointer and it avoids the leak. While I was there I also removed the casts of the kmalloc() return value since it's pointless. I also updated the driver version since this patch changes the workings of the code (however slightly). This issue could probably be fixed a lot more elegantly, but the code is a big mess IMHO and I just took the least intrusive route to a fix that I could find instead of starting on a cleanup as well (that can come later). Please consider for inclusion. Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com> Signed-off-by: James Bottomley <James.Bottomley@SteelEye.com>
Diffstat (limited to 'drivers/block/DAC960.c')
-rw-r--r--drivers/block/DAC960.c13
1 files changed, 7 insertions, 6 deletions
diff --git a/drivers/block/DAC960.c b/drivers/block/DAC960.c
index 45bcda544880..dd8a1501142f 100644
--- a/drivers/block/DAC960.c
+++ b/drivers/block/DAC960.c
@@ -17,8 +17,8 @@
17*/ 17*/
18 18
19 19
20#define DAC960_DriverVersion "2.5.47" 20#define DAC960_DriverVersion "2.5.48"
21#define DAC960_DriverDate "14 November 2002" 21#define DAC960_DriverDate "14 May 2006"
22 22
23 23
24#include <linux/module.h> 24#include <linux/module.h>
@@ -4780,15 +4780,16 @@ static void DAC960_V2_ProcessCompletedCommand(DAC960_Command_T *Command)
4780 (NewPhysicalDeviceInfo->LogicalUnit != 4780 (NewPhysicalDeviceInfo->LogicalUnit !=
4781 PhysicalDeviceInfo->LogicalUnit)) 4781 PhysicalDeviceInfo->LogicalUnit))
4782 { 4782 {
4783 PhysicalDeviceInfo = (DAC960_V2_PhysicalDeviceInfo_T *) 4783 PhysicalDeviceInfo =
4784 kmalloc(sizeof(DAC960_V2_PhysicalDeviceInfo_T), GFP_ATOMIC); 4784 kmalloc(sizeof(DAC960_V2_PhysicalDeviceInfo_T), GFP_ATOMIC);
4785 InquiryUnitSerialNumber = 4785 InquiryUnitSerialNumber =
4786 (DAC960_SCSI_Inquiry_UnitSerialNumber_T *)
4787 kmalloc(sizeof(DAC960_SCSI_Inquiry_UnitSerialNumber_T), 4786 kmalloc(sizeof(DAC960_SCSI_Inquiry_UnitSerialNumber_T),
4788 GFP_ATOMIC); 4787 GFP_ATOMIC);
4789 if (InquiryUnitSerialNumber == NULL && 4788 if (InquiryUnitSerialNumber == NULL ||
4790 PhysicalDeviceInfo != NULL) 4789 PhysicalDeviceInfo == NULL)
4791 { 4790 {
4791 kfree(InquiryUnitSerialNumber);
4792 InquiryUnitSerialNumber = NULL;
4792 kfree(PhysicalDeviceInfo); 4793 kfree(PhysicalDeviceInfo);
4793 PhysicalDeviceInfo = NULL; 4794 PhysicalDeviceInfo = NULL;
4794 } 4795 }