policy: kdbus_policy_set() fix a use after free bug
authorDjalal Harouni <tixxdz@opendz.org>
Fri, 20 Jun 2014 16:50:04 +0000 (17:50 +0100)
committerDaniel Mack <zonque@gmail.com>
Fri, 20 Jun 2014 17:31:04 +0000 (19:31 +0200)
If we try to register the same name twice then kdbus_policy_add_one()
will fail with -EEXIST

In kdbus_policy_set() we have two calls to kdbus_policy_add_one() if
they fail we clean things up with kdbus_policy_entry_free(), but since
we failed ret == -EEXIST ,we hit the error path where we have another:

if (e)
kdbus_policy_entry_free(e);

We have a use after free bug here, Since 'e' is freed but never set to
NULL.

To fix this we can set that 'e' to NULL after each
kdbus_policy_entry_free() call, but it is better to just clean things up
in a one place, in the error path and remove the other
kdbus_policy_entry_free() calls.

Thix fixes the bug triggered by test-kdbus-policy when we try to
register the same name twice.

Signed-off-by: Djalal Harouni <tixxdz@opendz.org>
policy.c

index 21e489af24fd9a777af6babf80630b0aba7a8339..20188c99705da58ed2b4ddfe155a519b9f4b15b3 100644 (file)
--- a/policy.c
+++ b/policy.c
@@ -510,10 +510,8 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
 
                        if (e) {
                                ret = kdbus_policy_add_one(db, e);
-                               if (ret < 0) {
-                                       kdbus_policy_entry_free(e);
+                               if (ret < 0)
                                        goto exit;
-                               }
                        }
 
                        if (max_policies && ++count > max_policies) {
@@ -582,11 +580,8 @@ int kdbus_policy_set(struct kdbus_policy_db *db,
                goto exit;
        }
 
-       if (e) {
+       if (e)
                ret = kdbus_policy_add_one(db, e);
-               if (ret < 0)
-                       kdbus_policy_entry_free(e);
-       }
 
 exit:
        if (ret < 0) {