Re: A little patch to clean up multiple declarations of the cli_ipcname functions
Roger Tsang <[email protected]> Thu, 14 Jan 2010 03:39:12 +0000
| Newsgroups | gmane.linux.cluster.ssic.devel |
|---|---|
| Message-ID | <[email protected]> |
On Wed, Jan 13, 2010 at 3:14 PM, John Hughes <[email protected]> wrote: > Ok, I've finished my patch to clean up the ipc directory, getting rid of > multiple dodgy "extern" declarations. > > Found a couple of bugs in the process (missing cprid argument from > ssi_shm_get_shmid_kernel, wrong type (int instead of u_long) for second > argument to cli_ipcname_findid). > Nice one. Can't believe we missed that bug with cprid argument. Some things I noticed with your patch: 1. Missing fix for "typedef u_long global_id_t" 2. Warning: trailing whitespace in line 110 of cluster/ssi/ipc/ipcshm_svr.c Also while looking I found another bug. There's a memory leak in ssi_shm_get_shmid_kernel() caller's error path. In fact there's a memory leak in all /proc/sysvipc/* read routines. Patch attached. -Roger ------------------------------------------------------------------------------ Throughout its 18-year history, RSA Conference consistently attracts the world's best and brightest in the field, creating opportunities for Conference attendees to learn about information security's most important issues through interactions with peers, luminaries and emerging and established companies. http://p.sf.net/sfu/rsaconf-dev2dev _______________________________________________ ssic-linux-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ssic-linux-devel
sysvipc_read_memory_leak.patch
(text/x-patch, 2.3 KB)
IPC:
- Fix sysvipc_msg_read_proc() leaking msg_queue struct when ssi_msg_get_msg_queue() fails.
- Fix sysvipc_sem_read_proc() leaking sem_array struct when ssi_sem_get_sem_array() fails.
- Fix sysvipc_shm_read_proc() leaking shmid_kernel struct when ssi_shm_get_shmid_kernel() fails.
Index: linux/ipc/msg.c
===================================================================
--- linux.orig/ipc/msg.c
+++ linux/ipc/msg.c
@@ -1508,10 +1508,15 @@ static int sysvipc_msg_read_proc(char *b
continue;
} else if (node_num == this_node) {
msq = msg_lock(ipc_id);
- } else if (!(msq = kmalloc(sizeof(*msq), GFP_KERNEL)) ||
- ssi_msg_get_msg_queue(node_num, ipc_id,
+ } else {
+ msq = kmalloc(sizeof(*msq), GFP_KERNEL);
+ if (msq == NULL)
+ continue;
+ if (ssi_msg_get_msg_queue(node_num, ipc_id,
(char **)&msq) != 0) {
- continue;
+ kfree(msq);
+ continue;
+ }
}
} else {
node_num = this_node;
Index: linux/ipc/shm.c
===================================================================
--- linux.orig/ipc/shm.c
+++ linux/ipc/shm.c
@@ -1755,10 +1755,16 @@ static int sysvipc_shm_read_proc(char *b
shp = shm_lock(ipc_id);
segsize = shm_get_segsize(shp);
cprid = shm_get_cpid(shp);
- } else if (!(shp = kmalloc(sizeof(*shp), GFP_KERNEL)) ||
- ssi_shm_get_shmid_kernel(node_num, ipc_id,
- (char **)&shp, &segsize, &cprid) != 0) {
- continue;
+ } else {
+ shp = kmalloc(sizeof(*shp), GFP_KERNEL);
+ if (shp == NULL)
+ continue;
+ if (ssi_shm_get_shmid_kernel(
+ node_num, ipc_id, (char **)&shp,
+ &segsize, &cprid) != 0) {
+ kfree(shp);
+ continue;
+ }
}
} else {
shp = shm_lock(i);
Index: linux/ipc/sem.c
===================================================================
--- linux.orig/ipc/sem.c
+++ linux/ipc/sem.c
@@ -2154,10 +2154,15 @@ static int sysvipc_sem_read_proc(char *b
continue;
} else if (node_num == this_node) {
sma = sem_lock(ipc_id);
- } else if (!(sma = kmalloc(sizeof(*sma), GFP_KERNEL)) ||
- ssi_sem_get_sem_array(node_num, ipc_id,
+ } else {
+ sma = kmalloc(sizeof(*sma), GFP_KERNEL);
+ if (sma == NULL)
+ continue;
+ if (ssi_sem_get_sem_array(node_num, ipc_id,
(char **) &sma) != 0) {
- continue;
+ kfree(sma);
+ continue;
+ }
}
} else {
sma = sem_lock(i);