Changeset 37

Timestamp:

05/30/08 17:23:36 (6 months ago)

Author:

jtang

Message:

Fixed buffer overflows in message queue code.
Fixing race condition with shm, due to ordering of messages within side-channel queue.

Files:

• trunk/libsipc/src/mqueue_internal.c (modified) (1 diff)
• trunk/libsipc/src/shm_internal.c (modified) (1 diff)
• trunk/libsipc/src/sipc_internal.h (modified) (1 diff)
• trunk/libsipc/src/sipc_mqueue.c (modified) (3 diffs)
• trunk/libsipc/src/sipc_shm.c (modified) (6 diffs)
• trunk/libsipc/tests/mqueue.c (modified) (4 diffs)
• trunk/libsipc/tests/shm.c (modified) (4 diffs)

trunk/libsipc/src/mqueue_internal.c

@@ -123 +123 @@
-                if (errno == ENOMSG)
-                        return 0;
+                return -1;
-        }
-

trunk/libsipc/src/shm_internal.c

@@ -2 +2 @@
-*
-* Copyright (C) 2006, 2007 Tresys Technology, LLC
- 
+
-*
-*  This library is free software; you can redistribute it and/or

trunk/libsipc/src/sipc_internal.h

@@ -28 +28 @@
-
-/* Control message types */
-        
- 
-        
+       
+        
+       
-#define SIPC_MSG_LEN    0x09
-   
+  
-
-/* Invalid message length */

trunk/libsipc/src/sipc_mqueue.c

@@ -205 +205 @@
-        /* Receive and validate the length of message marker */
-        if (sipc->msg_len == SIPC_MSGLEN_NOT_SET) {
-
+ - sizeof(long)
-                        sipc_error(sipc, "msgrcv: %s\n", strerror(errno));
-                        goto err;
@@ -232 +232 @@
-         * Stop when we have received the total number of bytes
-         * or upon receiving the end of message marker. */
-
+ - sizeof(long)
-                sipc_error(sipc, "msgrcv: %s\n", strerror(errno));
-                goto err;
@@ -256 +256 @@
-                idx += recv_sz;
-
-                /* Check to see if we have received the entire message */
-                if (*len >= sipc->msg_len)
-                        break;
-
-                /* Receive the next message */
-
+ - sizeof(long)
-                        sipc_error(sipc, "msgrcv: %s\n", strerror(errno));
-                        goto err;

trunk/libsipc/src/sipc_shm.c

@@ -78 +78 @@
-                goto err;
-
-1 message
-sipc->role == SIPC_SENDER && mqueue_set_capacity(sipc->s.msqid, sizeof(struct msgbuf)
+3 messages
+mqueue_set_capacity(sipc->s.msqid, 3
-                goto err;
-
@@ -151 +151 @@
-}
-
-
+
+
+
+
+
+
+
+
+
+
+
+
+
-int sipc_shm_send_data(sipc_t *sipc, int msg_len)
-{
-        if (!sipc)
-                return -1;
+
+        /* Send a NULL marker; this should block at first */
+        if (mqueue_send_msg_type(sipc->s.msqid, SIPC_NOP, MQ_BLOCK) < 0) {
+                sipc_error(sipc, "Could not send NOP marker\n");
+                return -1;
+        }
-
-        /* Send a DATA_READY marker */
@@ -163 +181 @@
-        }
-
-        /* Send a NULL marker; this should block at first */
-        if (mqueue_send_msg_type(sipc->s.msqid, SIPC_NOP, MQ_BLOCK) < 0) {
-                sipc_error(sipc, "Could not send NOP marker\n");
-                return -1;
-        }
-
-        /* Send another NULL marker */
-        if (mqueue_send_msg_type(sipc->s.msqid, SIPC_NOP, MQ_BLOCK) < 0) {
@@ -179 +191 @@
-
-/* Receive a DATA_READY marker from sender and pass
- 
+
- * This function will block if the DATA_READY marker is not
- * present on the queue. */
@@ -189 +201 @@
-                return -1;
-
-
-
+
+
+
+
+
-        if (mtype == SIPC_DATA_READY) {
-                /* It is now OK to read shared memory */
@@ -196 +211 @@
-                *len = sipc->len;
-                return 0;
-
+if (mtype < 0) 
-                sipc_error(sipc, "Received a message of unknown type\n");
-                return -1;

trunk/libsipc/tests/mqueue.c

@@ -44 +44 @@
-static void do_parent(void);
-static void do_child(void);
+static int verify_data(char *filename, char *recv_data);
-static inline int is_end_xmit(char *data);
-
@@ -137 +138 @@
-        CU_ASSERT(sipc_send_data(writer_ipc, message_len) == 0);
-        CU_ASSERT(sipc_recv_data(reader_ipc, &recv_data, &recv_len) == 0);
+        CU_ASSERT_PTR_NOT_NULL_FATAL(recv_data);
-        CU_ASSERT(recv_len >= message_len);
-        CU_ASSERT(memcmp(recv_data, message, message_len) == 0);
+        free(recv_data);
-        CU_ASSERT(sipc_shm_recv_done(reader_ipc) == 0);
-}
@@ -178 +181 @@
-        int len = 0;
-
-        /* Get pointer to IPC's data member */
-        data = sipc_get_data_ptr(reader_ipc);
-        CU_ASSERT_PTR_NOT_NULL(data);
-
-        while (!sipc_recv_data(reader_ipc, &data, &len)) {
-
+
+
+
-                        break;
+                }
+                if (verify_data(IN_FILE, data) != 1) {
+                        CU_PASS("Sent data matches received data");
+                } else {
+                        CU_FAIL("Sent data does not match received data!");
+                }
+                free(data);
-        }
-}
@@ -237 +245 @@
-}
-
+/* Read DATA_LEN bytes from a file and determine if
+ * the read contents match the provided character string */
+static int verify_data(char *filename, char *recv_data)
+{
+        FILE *ifile = NULL;
+        char data[DATA_LEN];
+        int rbytes = 0;
+
+        ifile = fopen(filename, "r");
+        if (!ifile) {
+                fprintf(stderr, "fopen: %s\n", strerror(errno));
+                return -1;
+        }
+
+        rbytes = fread(data, sizeof(char), DATA_LEN - 1, ifile);
+        if (rbytes < 0) {
+                fclose(ifile);
+                return -1;
+        }
+        fclose(ifile);
+
+        if (memcmp(data, recv_data, rbytes))
+                return 1;
+
+        return -1;
+}
+
-static inline int is_end_xmit(char *data)
-{

trunk/libsipc/tests/shm.c

@@ -245 +245 @@
-        int len = 0;
-
+        memset(recv_data, 0, sizeof(recv_data));
-        while (!sipc_recv_data(reader_ipc, &data, &len)) {
+                CU_ASSERT_PTR_NOT_NULL_FATAL(data);
-                if (END_XMIT(data)) {
-                        sipc_shm_recv_done(reader_ipc);
@@ -252 +254 @@
-
-                /* Copy received data for verification later */
-strncpy(recv_data, data, DATA_LEN
+memcpy(recv_data, data, len
-                sipc_shm_recv_done(reader_ipc);
-        }
@@ -262 +264 @@
-                CU_FAIL("Sent data does not match received data!");
-        }
-
-}
-
@@ -301 +302 @@
-        int len = 0, i;
-
+        memset(recv_data, 0, sizeof(recv_data));
-        while (!sipc_recv_data(reader_ipc, &data, &len)) {
-                if (END_XMIT(data)) {