bus: support temporarily const errors that don't need to be freed but require deep copies
This should fix issues with incorrectly copying bus error messages out of sd_bus_message objects. Original bug found by: Djalal Harouni
This commit is contained in:
@@ -210,20 +210,20 @@ bool bus_error_is_dirty(sd_bus_error *e) {
|
||||
if (!e)
|
||||
return false;
|
||||
|
||||
return e->name || e->message || e->need_free;
|
||||
return e->name || e->message || e->_need_free != 0;
|
||||
}
|
||||
|
||||
_public_ void sd_bus_error_free(sd_bus_error *e) {
|
||||
if (!e)
|
||||
return;
|
||||
|
||||
if (e->need_free) {
|
||||
if (e->_need_free > 0) {
|
||||
free((void*) e->name);
|
||||
free((void*) e->message);
|
||||
}
|
||||
|
||||
e->name = e->message = NULL;
|
||||
e->need_free = false;
|
||||
e->_need_free = 0;
|
||||
}
|
||||
|
||||
_public_ int sd_bus_error_set(sd_bus_error *e, const char *name, const char *message) {
|
||||
@@ -244,7 +244,7 @@ _public_ int sd_bus_error_set(sd_bus_error *e, const char *name, const char *mes
|
||||
if (message)
|
||||
e->message = strdup(message);
|
||||
|
||||
e->need_free = true;
|
||||
e->_need_free = 1;
|
||||
|
||||
finish:
|
||||
return -bus_error_name_to_errno(name);
|
||||
@@ -268,7 +268,7 @@ int bus_error_setfv(sd_bus_error *e, const char *name, const char *format, va_li
|
||||
if (format)
|
||||
vasprintf((char**) &e->message, format, ap);
|
||||
|
||||
e->need_free = true;
|
||||
e->_need_free = 1;
|
||||
|
||||
finish:
|
||||
return -bus_error_name_to_errno(name);
|
||||
@@ -299,7 +299,13 @@ _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)
|
||||
/*
|
||||
* _need_free < 0 indicates that the error is temporarily const, needs deep copying
|
||||
* _need_free == 0 indicates that the error is perpetually const, needs no deep copying
|
||||
* _need_free > 0 indicates that the error is fully dynamic, needs deep copying
|
||||
*/
|
||||
|
||||
if (e->_need_free == 0)
|
||||
*dest = *e;
|
||||
else {
|
||||
dest->name = strdup(e->name);
|
||||
@@ -311,7 +317,7 @@ _public_ int sd_bus_error_copy(sd_bus_error *dest, const sd_bus_error *e) {
|
||||
if (e->message)
|
||||
dest->message = strdup(e->message);
|
||||
|
||||
dest->need_free = true;
|
||||
dest->_need_free = 1;
|
||||
}
|
||||
|
||||
finish:
|
||||
@@ -380,7 +386,7 @@ static void bus_error_strerror(sd_bus_error *e, int error) {
|
||||
}
|
||||
|
||||
if (x == m) {
|
||||
if (e->need_free) {
|
||||
if (e->_need_free > 0) {
|
||||
/* Error is already dynamic, let's just update the message */
|
||||
free((char*) e->message);
|
||||
e->message = x;
|
||||
@@ -395,14 +401,14 @@ static void bus_error_strerror(sd_bus_error *e, int error) {
|
||||
return;
|
||||
}
|
||||
|
||||
e->need_free = true;
|
||||
e->_need_free = 1;
|
||||
e->name = t;
|
||||
e->message = x;
|
||||
}
|
||||
} else {
|
||||
free(m);
|
||||
|
||||
if (e->need_free) {
|
||||
if (e->_need_free > 0) {
|
||||
char *t;
|
||||
|
||||
/* Error is dynamic, let's hence make the message also dynamic */
|
||||
@@ -444,7 +450,7 @@ _public_ int sd_bus_error_set_errno(sd_bus_error *e, int error) {
|
||||
|
||||
k = errno_to_bus_error_name_new(error, (char**) &e->name);
|
||||
if (k > 0)
|
||||
e->need_free = true;
|
||||
e->_need_free = 1;
|
||||
else if (k < 0) {
|
||||
*e = BUS_ERROR_OOM;
|
||||
return -error;
|
||||
@@ -480,7 +486,7 @@ int bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_lis
|
||||
|
||||
k = errno_to_bus_error_name_new(error, (char**) &e->name);
|
||||
if (k > 0)
|
||||
e->need_free = true;
|
||||
e->_need_free = 1;
|
||||
else if (k < 0) {
|
||||
*e = BUS_ERROR_OOM;
|
||||
return -ENOMEM;
|
||||
@@ -496,12 +502,12 @@ int bus_error_set_errnofv(sd_bus_error *e, int error, const char *format, va_lis
|
||||
r = vasprintf(&m, format, ap);
|
||||
if (r >= 0) {
|
||||
|
||||
if (!e->need_free) {
|
||||
if (e->_need_free <= 0) {
|
||||
char *t;
|
||||
|
||||
t = strdup(e->name);
|
||||
if (t) {
|
||||
e->need_free = true;
|
||||
e->_need_free = 1;
|
||||
e->name = t;
|
||||
e->message = m;
|
||||
return -error;
|
||||
|
||||
@@ -609,6 +609,8 @@ _public_ int sd_bus_message_new_method_error(
|
||||
goto fail;
|
||||
}
|
||||
|
||||
t->error._need_free = -1;
|
||||
|
||||
*m = t;
|
||||
return 0;
|
||||
|
||||
@@ -709,6 +711,8 @@ int bus_message_new_synthetic_error(
|
||||
goto fail;
|
||||
}
|
||||
|
||||
t->error._need_free = -1;
|
||||
|
||||
*m = t;
|
||||
return 0;
|
||||
|
||||
@@ -3804,6 +3808,9 @@ int bus_message_parse_fields(sd_bus_message *m) {
|
||||
return -EBADMSG;
|
||||
|
||||
r = message_peek_field_string(m, error_name_is_valid, &ri, &m->error.name);
|
||||
if (r >= 0)
|
||||
m->error._need_free = -1;
|
||||
|
||||
break;
|
||||
|
||||
case SD_BUS_MESSAGE_HEADER_DESTINATION:
|
||||
|
||||
@@ -26,6 +26,12 @@
|
||||
int main(int argc, char *argv[]) {
|
||||
|
||||
_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL, second = SD_BUS_ERROR_NULL;
|
||||
const sd_bus_error const_error = SD_BUS_ERROR_MAKE_CONST(SD_BUS_ERROR_FILE_EXISTS, "const error");
|
||||
const sd_bus_error temporarily_const_error = {
|
||||
.name = SD_BUS_ERROR_ACCESS_DENIED,
|
||||
.message = "oh! no",
|
||||
._need_free = -1
|
||||
};
|
||||
|
||||
assert_se(!sd_bus_error_is_set(&error));
|
||||
assert_se(sd_bus_error_set(&error, SD_BUS_ERROR_NOT_SUPPORTED, "xxx") == -ENOTSUP);
|
||||
@@ -45,7 +51,10 @@ int main(int argc, char *argv[]) {
|
||||
assert_se(sd_bus_error_is_set(&error));
|
||||
|
||||
assert_se(!sd_bus_error_is_set(&second));
|
||||
assert_se(second._need_free == 0);
|
||||
assert_se(error._need_free > 0);
|
||||
assert_se(sd_bus_error_copy(&second, &error) == -ENOENT);
|
||||
assert_se(second._need_free > 0);
|
||||
assert_se(streq(error.name, second.name));
|
||||
assert_se(streq(error.message, second.message));
|
||||
assert_se(sd_bus_error_get_errno(&second) == ENOENT);
|
||||
@@ -53,6 +62,28 @@ int main(int argc, char *argv[]) {
|
||||
assert_se(sd_bus_error_is_set(&second));
|
||||
|
||||
sd_bus_error_free(&error);
|
||||
sd_bus_error_free(&second);
|
||||
|
||||
assert_se(!sd_bus_error_is_set(&second));
|
||||
assert_se(const_error._need_free == 0);
|
||||
assert_se(sd_bus_error_copy(&second, &const_error) == -EEXIST);
|
||||
assert_se(second._need_free == 0);
|
||||
assert_se(streq(const_error.name, second.name));
|
||||
assert_se(streq(const_error.message, second.message));
|
||||
assert_se(sd_bus_error_get_errno(&second) == EEXIST);
|
||||
assert_se(sd_bus_error_has_name(&second, SD_BUS_ERROR_FILE_EXISTS));
|
||||
assert_se(sd_bus_error_is_set(&second));
|
||||
sd_bus_error_free(&second);
|
||||
|
||||
assert_se(!sd_bus_error_is_set(&second));
|
||||
assert_se(temporarily_const_error._need_free < 0);
|
||||
assert_se(sd_bus_error_copy(&second, &temporarily_const_error) == -EACCES);
|
||||
assert_se(second._need_free > 0);
|
||||
assert_se(streq(temporarily_const_error.name, second.name));
|
||||
assert_se(streq(temporarily_const_error.message, second.message));
|
||||
assert_se(sd_bus_error_get_errno(&second) == EACCES);
|
||||
assert_se(sd_bus_error_has_name(&second, SD_BUS_ERROR_ACCESS_DENIED));
|
||||
assert_se(sd_bus_error_is_set(&second));
|
||||
|
||||
assert_se(!sd_bus_error_is_set(&error));
|
||||
assert_se(sd_bus_error_set_const(&error, "Posix.Error.EUCLEAN", "Hallo") == -EUCLEAN);
|
||||
|
||||
@@ -42,7 +42,7 @@ typedef struct sd_bus_creds sd_bus_creds;
|
||||
typedef struct {
|
||||
const char *name;
|
||||
const char *message;
|
||||
int need_free;
|
||||
int _need_free;
|
||||
} sd_bus_error;
|
||||
|
||||
/* Flags */
|
||||
|
||||
Reference in New Issue
Block a user