bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 1/3] ipc: avoid dereference of null pointer and quiet the GCC war


From: Marin Ramesa
Subject: [PATCH 1/3] ipc: avoid dereference of null pointer and quiet the GCC warning about uninitialized variable
Date: Wed, 18 Dec 2013 09:17:47 +0100

The problem is in this statement:

assert((entry == IE_NULL) || IE_BITS_TYPE(entry->ie_bits));

The macro assert() checks for a negation of this expression. Negation of an
OR expression is an AND expression. In order to evaluate an AND expression,
compiler needs to check both conditions. So it first evaluates (entry == 
IE_NULL)
and then IE_BITS_TYPE(entry->ie_bits). If the code path was that where entry is 
a
null pointer, the evaluation of the second condition results in a dereference 
of a null
pointer. Since this dereference happend, the initialization of notify_port is 
never
reached in this code path, and GCC concludes that notify_port is later used 
uninitialized.

* ipc/ipc_entry.c (ipc_entry_lookup) (assert): Fix assertion.
* ipc/ipc_kmsg.c (ipc_kmsg_copyin_header) (notify_port): Don't initialize to 
zero.
(ipc_kmsg_copyin_header): Break the if statement into two statements to avoid 
having a check for null pointer and dereference in one statement. 

---
 ipc/ipc_entry.c | 2 +-
 ipc/ipc_kmsg.c  | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/ipc/ipc_entry.c b/ipc/ipc_entry.c
index f1d763c..f182baf 100644
--- a/ipc/ipc_entry.c
+++ b/ipc/ipc_entry.c
@@ -123,7 +123,7 @@ ipc_entry_lookup(space, name)
                entry = (ipc_entry_t)
                                ipc_splay_tree_lookup(&space->is_tree, name);
 
-       assert((entry == IE_NULL) || IE_BITS_TYPE(entry->ie_bits));
+       assert((entry == IE_NULL) ||  ((entry != IE_NULL) ? 
IE_BITS_TYPE(entry->ie_bits) : TRUE));
        return entry;
 }
 
diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c
index 0e43410..270d511 100644
--- a/ipc/ipc_kmsg.c
+++ b/ipc/ipc_kmsg.c
@@ -944,7 +944,7 @@ ipc_kmsg_copyin_header(msg, space, notify)
        mach_msg_type_name_t reply_type = MACH_MSGH_BITS_LOCAL(mbits);
        ipc_object_t dest_port, reply_port;
        ipc_port_t dest_soright, reply_soright;
-       ipc_port_t notify_port = 0; /* '=0' to quiet gcc warnings */
+       ipc_port_t notify_port;
 
        if (!MACH_MSG_TYPE_PORT_ANY_SEND(dest_type))
                return MACH_SEND_INVALID_HEADER;
@@ -961,8 +961,11 @@ ipc_kmsg_copyin_header(msg, space, notify)
        if (notify != MACH_PORT_NULL) {
                ipc_entry_t entry;
 
-               if (((entry = ipc_entry_lookup(space, notify)) == IE_NULL) ||
-                   ((entry->ie_bits & MACH_PORT_TYPE_RECEIVE) == 0)) {
+               if ((entry = ipc_entry_lookup(space, notify)) == IE_NULL) {
+                       is_write_unlock(space);
+                       return MACH_SEND_INVALID_NOTIFY;
+               }
+               if ((entry->ie_bits & MACH_PORT_TYPE_RECEIVE) == 0) {
                        is_write_unlock(space);
                        return MACH_SEND_INVALID_NOTIFY;
                }
-- 
1.8.1.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]