[PATCH libdmraid-events 0/3] Assorted fixes

[PATCH libdmraid-events 0/3] Assorted fixes

[NeilBrown]

Hi,
 a recent code inspection found a few minor bugs in libdmraid-events. 
 Here are 3 patches which address these issues.
 This is based on 1.0.0.rc4 retrieved from

http://sources.redhat.com/lvm2/wiki/DMRAID_Eventing?action=AttachFile&do=get&target=libdmraid-events_DSO-1.0.0.rc4.tgz

Thanks,
NeilBrown

[PATCH libdmraid-events 1/3] Add some missing closedir calls.

[NeilBrown]

[-- Attachment #1: closedir-fixes --]
[-- Type: text/plain, Size: 990 bytes --]

Two times opendir is called without a matching
closedir - so add those calls.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 libdmraid-events.c |    2 ++
 1 file changed, 2 insertions(+)

--- libdmraid-events.orig/libdmraid-events.c
+++ libdmraid-events/libdmraid-events.c
@@ -172,6 +172,7 @@ static int _repopulate(const char* devic
 						memcpy(test_path+strlen(sys_path), strchr(disk+1, '/')+1, strlen(strchr(disk+1, '/')+1));
 						if((dir = opendir(test_path)))
 						{
+							closedir(dir);
 							/* test path is for this raid volume */
 							memcpy(test_path+strlen(test_path), "/dev", 4);
 							if(!(fd = fopen(test_path, "r"))) {
@@ -959,6 +960,7 @@ int register_device(const char *device, 
 					if(!(dir = opendir(test_path)))
 						continue;
 					else {
+						closedir(dir);
 						/* test path is for this raid volume */
 						memcpy(rv_next->raid_mem[m].dev_name, scsi_dev, strlen(scsi_dev));
 						memset(rv_next->raid_mem[m].dev_name+strlen(scsi_dev), 0, 1);

[PATCH libdmraid-events 2/3] Fix some issues with use of snprintf

[NeilBrown]

[-- Attachment #1: snprint.fixes --]
[-- Type: text/plain, Size: 2140 bytes --]

Several times, the buf len argument passed to snprintf is a
'sizeof' something which is only vaguely related to the size of the buffer,
and in some cases is definitely larger than the buffer.
Also snprintf does not guarantee to produce a nul terminated string if
an overflow occurs.

So pass more appropriate buffer lengths and ensure result is nul
terminated.
Also make 'dm_num' large enough to hold more than a single digit.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 libdmraid-events.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

--- libdmraid-events.orig/libdmraid-events.c
+++ libdmraid-events/libdmraid-events.c
@@ -303,6 +303,7 @@ static char *_match_port(const char *vol
 			for(i = 0; i < curr->num_sata_drives; i++) 
 				if(curr->raid_mem[i].port_num>=0){
 				snprintf(port, 4, "%d", curr->raid_mem[i].port_num);
+				port[4] = 0;
 				memcpy(buf+strlen(buf), curr->raid_mem[i].dev_name, strlen(curr->raid_mem[i].dev_name));
 				memcpy(buf+strlen(buf), "=", 1);
 				memcpy(buf+strlen(buf), port, strlen(port));
@@ -378,7 +379,7 @@ static char *_get_dev_names(const char *
 	int num = 0;
 	FILE *fd;
 	char sys_path[BUF_SIZE];
-	char dm_num[2];
+	char dm_num[5];
 	char dm_mm[MAJOR_MINOR];
 	char f_mm[MAJOR_MINOR];
 	struct dm_task *dmt;
@@ -406,8 +407,10 @@ static char *_get_dev_names(const char *
 	memset(sys_path, 0, BUF_SIZE);
 	memcpy(sys_path, SYS_DM_PATH, strlen(SYS_DM_PATH));
 	memset(dm_mm, 0, MAJOR_MINOR);
-	snprintf(dm_mm, sizeof(info.major)+sizeof(info.minor), "%d:%d", info.major, info.minor);
-	snprintf(dm_num, sizeof(num = 0), "%d", num);
+	snprintf(dm_mm, MAJOR_MINOR-1, "%d:%d", info.major, info.minor);
+	num = 0;
+	snprintf(dm_num, sizeof(dm_num), "%d", num);
+
 			
 	while(!access(strncat(sys_path, dm_num, strlen(dm_num)), F_OK)) {
 				
@@ -423,7 +426,8 @@ static char *_get_dev_names(const char *
 			/* Reset string for next iteration */
 			memset(sys_path+strlen(SYS_DM_PATH), 0, 1);
 			num++;
-			snprintf(dm_num, sizeof(num), "%d", num);					
+			snprintf(dm_num, sizeof(dm_num), "%d", num);
+			dm_num[sizeof(dm_num)-1] = 0;
 			fclose(fd);
 			continue;
 		}

[PATCH libdmraid-events 3/3] Fix minor memory leak in lib_main

[NeilBrown]

[-- Attachment #1: memory-leak --]
[-- Type: text/plain, Size: 735 bytes --]

As lib_argv[3] is set to NULL, there is no point mallocing
some space an putting it there.

Signed-off-by: NeilBrown <neilb@suse.de>
---
 libdmraid-events.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- libdmraid-events.orig/libdmraid-events.c
+++ libdmraid-events/libdmraid-events.c
@@ -209,7 +209,7 @@ static int lib_main(char op, const char*
     int lib_argc=3;
     char *lib_argv[4];
     
-    for(i=0; i<4; i++)
+    for(i=0; i<3; i++)
 		lib_argv[i]=malloc(strlen(device) + 1);
     
     strcpy(lib_argv[0], "dso");
@@ -229,7 +229,7 @@ static int lib_main(char op, const char*
 	libdmraid_exit(lc);
     }
 
-    for(i=0; i<4; i++)
+    for(i=0; i<3; i++)
 		free(lib_argv[i]);        
 
     return ret;

Re: [PATCH libdmraid-events 0/3] Assorted fixes

[Heinz Mauelshagen]

Neil,

you seem to be based on an old version of libdmraid-events.c.

Please rebase to sources.redhat.com:/cvs/dm/ (dmraid subdirectory is
recent), which most likely has all of that addressed already and rebase
if you still find the issues.

Thanks,
Heinz

On Fri, 2009-12-11 at 14:22 +1100, NeilBrown wrote:
> Hi,
>  a recent code inspection found a few minor bugs in libdmraid-events. 
>  Here are 3 patches which address these issues.
>  This is based on 1.0.0.rc4 retrieved from
> 
> http://sources.redhat.com/lvm2/wiki/DMRAID_Eventing?action=AttachFile&do=get&target=libdmraid-events_DSO-1.0.0.rc4.tgz
> 
> Thanks,
> NeilBrown
> 
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH libdmraid-events 0/3] Assorted fixes

[Neil Brown]

On Mon, 14 Dec 2009 15:28:50 +0100
Heinz Mauelshagen <heinzm@redhat.com> wrote:

> Neil,
> 
> you seem to be based on an old version of libdmraid-events.c.
> 
> Please rebase to sources.redhat.com:/cvs/dm/ (dmraid subdirectory is
> recent), which most likely has all of that addressed already and rebase
> if you still find the issues.
> 

Thanks for the pointer.
However I cannot find libdmraid-events.c in the repository.
Only libdmraid-events-isw.c which seems to be a very different thing

Is there still a libdmraid-events.c ? or has it been incorporated in
something else?

Thanks,
NeilBrown

Re: [PATCH libdmraid-events 0/3] Assorted fixes

[Neil Brown]

On Mon, 14 Dec 2009 15:28:50 +0100
Heinz Mauelshagen <heinzm@redhat.com> wrote:

> Neil,
> 
> you seem to be based on an old version of libdmraid-events.c.
> 
> Please rebase to sources.redhat.com:/cvs/dm/ (dmraid subdirectory is
> recent), which most likely has all of that addressed already and rebase
> if you still find the issues.
> 
>

Also, I tried building from that source tree and there seem to be a number
of problems.
 - include/dmraid/dmreg.h is needed but not provided
 - lib/dmraid.a needs lib/device/partition.o, but there is no rule for
   building that file.
 - when tools/dmraid is build, the "-I" flags don't get passed to 'cc' so the
   compile fails ... I haven't managed to figure out why that is yet.

Thanks,
NeilBrown