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

[Djalal Harouni]

Hi list,

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.

An example: the libsystemd-bus/sd-bus.c:sd_bus_call() function is using
an sd_bus_message *incoming heap pointer inside the for() loop.

If bus_read_message() succeed it will set the 'incoming' sd_bus_message
and if we have a match and the header type is an
SD_BUS_MESSAGE_METHOD_ERROR then sd_bus_error_copy() will get called to
copy the 'incoming->error' into the 'sd_bus_error *error' passed as an
argument to the sd_bus_call() function. Since 'incoming->error.need_free'
is false then destination's fields will be just set to source's fields.

Memory pointed by source's fields may change at any time! There is no
guarentee that this memory will be available to be used or processed
later by the sd_bus_error destination. Even the 'incoming' memory will
be freed right after the sd_bus_error_copy() by sd_bus_message_unref()
and at this point, the 'error' data might be invalid

I've experienced truncated error messsages, but not always! it is
related to lower layers and how bus messages are processed...

Note: sd_bus_error_copy() behavior was to always strdup() source, it was
changed in part by commit 780896a4f1ec


Fix:
1) make sd_bus_error_copy() always strdup content of source regardless
of the 'need_free' flag since it is never set. This will restore
behavior before commit 780896a4f1ec. This is the easy one.

The following patch just use this one.


2) Add a new sd_bus_error_strdup() API to force strdup() and make
sd_bus_error_copy() use it ?


I'm open to propostions, I can write another patch. Thanks!