[systemd-devel] [PATCH 1/1] bus: make sure we always return valid error messages

[Djalal Harouni]

The sd_bus_error_copy() is used by bus clients to copy sd_bus_error
containt. If the 'need_free' field of the sd_bus_error struct of the
source is false, then strdup() will be avoided and the 'name' and
'message' pointer fields of the destination are set to the sources's
fields. This will be true for all bus clients.

Callers of sd_bus_error_copy() do not guarentee that memory pointed by
sd_bus_error source will still be available for upper code which will
use it via the sd_bus_error destination to report errors, hence we might
endup returning invalid data.

>From a client perspective the use of the 'need_free' for const error
messages is limitted, and since sd_bus_error_copy() is currently only
used by those clients, remove the 'need_free' check and force the
strdup() for proper error reporting.

Actually this restores the old behavior that was modified in part by
commit 780896a4f1ec
---
 src/libsystemd-bus/bus-error.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/libsystemd-bus/bus-error.c b/src/libsystemd-bus/bus-error.c
index 968e80d..90a762e 100644
--- a/src/libsystemd-bus/bus-error.c
+++ b/src/libsystemd-bus/bus-error.c
@@ -291,6 +291,7 @@ _public_ int sd_bus_error_setf(sd_bus_error *e, const char *name, const char *fo
 }
 
 _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) {
+        char *x, *y = NULL;
 
         if (!sd_bus_error_is_set(e))
                 return 0;
@@ -299,21 +300,22 @@ _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) {
 
         assert_return(!bus_error_is_dirty(dest), -EINVAL);
 
-        if (!e->need_free)
-                *dest = *e;
-        else {
-                dest->name = strdup(e->name);
-                if (!dest->name) {
-                        *dest = BUS_ERROR_OOM;
+        *dest = BUS_ERROR_OOM;
+        x = strdup(e->name);
+        if (!x)
+                return -ENOMEM;
+
+        if (e->message) {
+                y = strdup(e->message);
+                if (!y) {
+                        free(x);
                         return -ENOMEM;
                 }
-
-                if (e->message)
-                        dest->message = strdup(e->message);
-
-                dest->need_free = true;
         }
 
+        dest->name = x;
+        dest->message = y;
+        dest->need_free = true;
 finish:
         return -bus_error_name_to_errno(e->name);
 }
-- 
1.8.3.1