From: Lennart Poettering Date: Wed, 15 May 2013 00:56:45 +0000 (+0200) Subject: bus: rework message struct to keep header with fields in same malloc() block X-Git-Tag: v205~244 X-Git-Url: http://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=c91cb83c4dbc9a95a0a2a38e7028e0a7f4af3b35 bus: rework message struct to keep header with fields in same malloc() block This allows us to guarantee that the first payload_vec we pass to the kernel for each message is guaranteed to include the full header and all its field. --- diff --git a/src/libsystemd-bus/bus-kernel.c b/src/libsystemd-bus/bus-kernel.c index 3aa408414..81469365f 100644 --- a/src/libsystemd-bus/bus-kernel.c +++ b/src/libsystemd-bus/bus-kernel.c @@ -211,7 +211,7 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) { sz = offsetof(struct kdbus_msg, items); /* Add in fixed header, fields header and payload */ - sz += (1 + !!m->fields + m->n_body_parts) * + sz += (1 + m->n_body_parts) * ALIGN8(offsetof(struct kdbus_item, vec) + sizeof(struct kdbus_vec)); /* Add space for bloom filter */ @@ -249,11 +249,7 @@ static int bus_message_setup_kmsg(sd_bus *b, sd_bus_message *m) { if (well_known) append_destination(&d, m->destination, dl); - append_payload_vec(&d, m->header, sizeof(*m->header)); - - if (m->fields) - append_payload_vec(&d, m->fields, ALIGN8(m->header->fields_size)); - + append_payload_vec(&d, m->header, BUS_MESSAGE_BODY_BEGIN(m)); MESSAGE_FOREACH_PART(part, i, m) append_payload_vec(&d, part->data, part->size); @@ -398,22 +394,6 @@ static void close_kdbus_msg(sd_bus *bus, struct kdbus_msg *k) { } } -static bool range_contains( - size_t astart, size_t asize, - size_t bstart, size_t bsize, - void *a, void **b) { - - if (bstart < astart) - return false; - - if (bstart + bsize > astart + asize) - return false; - - *b = (uint8_t*) a + (bstart - astart); - - return true; -} - static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k, sd_bus_message **ret) { sd_bus_message *m = NULL; struct kdbus_item *d; @@ -439,10 +419,10 @@ static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k, sd_bus_mess if (d->type == KDBUS_MSG_PAYLOAD_VEC) { if (!h) { - if (d->vec.size < sizeof(struct bus_header)) - return -EBADMSG; - h = UINT64_TO_PTR(d->vec.address); + + if (!bus_header_is_complete(h, d->vec.size)) + return -EBADMSG; } n_payload++; @@ -470,7 +450,7 @@ static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k, sd_bus_mess if (!h) return -EBADMSG; - r = bus_header_size(h, &total); + r = bus_header_message_size(h, &total); if (r < 0) return r; @@ -489,11 +469,7 @@ static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k, sd_bus_mess if (d->type == KDBUS_MSG_PAYLOAD_VEC) { size_t begin_body; - /* Fill in fields material */ - range_contains(idx, d->vec.size, ALIGN8(sizeof(struct bus_header)), BUS_MESSAGE_FIELDS_SIZE(m), - UINT64_TO_PTR(d->vec.address), &m->fields); - - begin_body = ALIGN8(sizeof(struct bus_header)) + ALIGN8(BUS_MESSAGE_FIELDS_SIZE(m)); + begin_body = BUS_MESSAGE_BODY_BEGIN(m); if (idx + d->vec.size > begin_body) { struct bus_body_part *part; @@ -507,10 +483,10 @@ static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k, sd_bus_mess } if (idx >= begin_body) { - part->data = (void*) d->vec.address; + part->data = UINT64_TO_PTR(d->vec.address); part->size = d->vec.size; } else { - part->data = (uint8_t*) (uintptr_t) d->vec.address + (begin_body - idx); + part->data = (uint8_t*) UINT64_TO_PTR(d->vec.address) + (begin_body - idx); part->size = d->vec.size - (begin_body - idx); } @@ -551,11 +527,6 @@ static int bus_kernel_make_message(sd_bus *bus, struct kdbus_msg *k, sd_bus_mess log_debug("Got unknown field from kernel %llu", d->type); } - if ((BUS_MESSAGE_FIELDS_SIZE(m) > 0 && !m->fields)) { - sd_bus_message_unref(m); - return -EBADMSG; - } - r = bus_message_parse_fields(m); if (r < 0) { sd_bus_message_unref(m); diff --git a/src/libsystemd-bus/bus-message.c b/src/libsystemd-bus/bus-message.c index 3790102a7..c1e1c468b 100644 --- a/src/libsystemd-bus/bus-message.c +++ b/src/libsystemd-bus/bus-message.c @@ -116,9 +116,6 @@ static void message_free(sd_bus_message *m) { if (m->free_header) free(m->header); - if (m->free_fields) - free(m->fields); - message_reset_parts(m); if (m->free_kdbus) @@ -151,66 +148,64 @@ static void message_free(sd_bus_message *m) { free(m); } -static void* buffer_extend(void **p, uint32_t *sz, size_t align, size_t extend) { - size_t start, end; - void *k; - - assert(p); - assert(sz); - assert(align > 0); - - start = ALIGN_TO((size_t) *sz, align); - end = start + extend; - - if (end == *sz) - return (uint8_t*) *p + start; +static void *message_extend_fields(sd_bus_message *m, size_t align, size_t sz) { + void *op, *np; + size_t old_size, new_size, start; - if (end > (size_t) ((uint32_t) -1)) - return NULL; + assert(m); - k = realloc(*p, end); - if (!k) + if (m->poisoned) return NULL; - /* Zero out padding */ - if (start > *sz) - memset((uint8_t*) k + *sz, 0, start - *sz); + old_size = sizeof(struct bus_header) + m->header->fields_size; + start = ALIGN_TO(old_size, align); + new_size = start + sz; - *p = k; - *sz = end; + if (old_size == new_size) + return (uint8_t*) m->header + old_size; - return (uint8_t*) k + start; -} + if (new_size > (size_t) ((uint32_t) -1)) + goto poison; -static void *message_extend_fields(sd_bus_message *m, size_t align, size_t sz) { - void *p, *op; - size_t os; + if (m->free_header) { + np = realloc(m->header, ALIGN8(new_size)); + if (!np) + goto poison; + } else { + /* Initially, the header is allocated as part of of + * the sd_bus_message itself, let's replace it by + * dynamic data */ - assert(m); + np = malloc(ALIGN8(new_size)); + if (!np) + goto poison; - if (m->poisoned) - return NULL; + memcpy(np, m->header, sizeof(struct bus_header)); + } - op = m->fields; - os = m->header->fields_size; + /* Zero out padding */ + if (start > old_size) + memset((uint8_t*) np + old_size, 0, start - old_size); - p = buffer_extend(&m->fields, &m->header->fields_size, align, sz); - if (!p) { - m->poisoned = true; - return NULL; - } + op = m->header; + m->header = np; + m->header->fields_size = new_size - sizeof(struct bus_header); /* Adjust quick access pointers */ - m->path = adjust_pointer(m->path, op, os, m->fields); - m->interface = adjust_pointer(m->interface, op, os, m->fields); - m->member = adjust_pointer(m->member, op, os, m->fields); - m->destination = adjust_pointer(m->destination, op, os, m->fields); - m->sender = adjust_pointer(m->sender, op, os, m->fields); - m->error.name = adjust_pointer(m->error.name, op, os, m->fields); + m->path = adjust_pointer(m->path, op, old_size, m->header); + m->interface = adjust_pointer(m->interface, op, old_size, m->header); + m->member = adjust_pointer(m->member, op, old_size, m->header); + m->destination = adjust_pointer(m->destination, op, old_size, m->header); + m->sender = adjust_pointer(m->sender, op, old_size, m->header); + m->error.name = adjust_pointer(m->error.name, op, old_size, m->header); - m->free_fields = true; + m->free_header = true; - return p; + return (uint8_t*) np + start; + +poison: + m->poisoned = true; + return NULL; } static int message_append_field_string( @@ -390,8 +385,6 @@ int bus_message_from_malloc( goto fail; } - m->fields = (uint8_t*) buffer + sizeof(struct bus_header); - m->n_body_parts = 1; m->body.data = (uint8_t*) buffer + sizeof(struct bus_header) + ALIGN8(BUS_MESSAGE_FIELDS_SIZE(m)); m->body.size = length - sizeof(struct bus_header) - ALIGN8(BUS_MESSAGE_FIELDS_SIZE(m)); @@ -3132,7 +3125,7 @@ static int message_peek_fields( assert(rindex); assert(align > 0); - return buffer_peek(m->fields, BUS_MESSAGE_FIELDS_SIZE(m), rindex, align, nbytes, ret); + return buffer_peek(BUS_MESSAGE_FIELDS(m), BUS_MESSAGE_FIELDS_SIZE(m), rindex, align, nbytes, ret); } static int message_peek_field_uint32( @@ -3584,21 +3577,13 @@ int bus_message_seal(sd_bus_message *m, uint64_t serial) { return r; } + /* Add padding at the end, since we know the body + * needs to start at an 8 byte alignment. */ + l = BUS_MESSAGE_FIELDS_SIZE(m); a = ALIGN8(l) - l; - - if (a > 0) { - /* Add padding at the end, since we know the body - * needs to start at an 8 byte alignment. */ - void *p; - - p = message_extend_fields(m, 1, a); - if (!p) - return -ENOMEM; - - memset(p, 0, a); - m->header->fields_size -= a; - } + if (a > 0) + memset((uint8_t*) BUS_MESSAGE_FIELDS(m) + l, 0, a); MESSAGE_FOREACH_PART(part, i, m) if (part->memfd >= 0 && !part->sealed) { @@ -3899,15 +3884,7 @@ int bus_message_get_blob(sd_bus_message *m, void **buffer, size_t *sz) { if (!p) return -ENOMEM; - e = mempcpy(p, m->header, sizeof(*m->header)); - - if (m->fields) { - e = mempcpy(e, m->fields, m->header->fields_size); - - if (m->header->fields_size % 8 != 0) - e = mempset(e, 0, 8 - (m->header->fields_size % 8)); - } - + e = mempcpy(p, m->header, BUS_MESSAGE_BODY_BEGIN(m)); MESSAGE_FOREACH_PART(part, i, m) e = mempcpy(e, part->data, part->size); @@ -3981,7 +3958,22 @@ const char* bus_message_get_arg(sd_bus_message *m, unsigned i) { return t; } -int bus_header_size(struct bus_header *h, size_t *sum) { +bool bus_header_is_complete(struct bus_header *h, size_t size) { + size_t full; + + assert(h); + assert(size); + + if (size < sizeof(struct bus_header)) + return false; + + full = sizeof(struct bus_header) + + (h->endian == SD_BUS_NATIVE_ENDIAN ? h->fields_size : bswap_32(h->fields_size)); + + return size >= full; +} + +int bus_header_message_size(struct bus_header *h, size_t *sum) { size_t fs, bs; assert(h); diff --git a/src/libsystemd-bus/bus-message.h b/src/libsystemd-bus/bus-message.h index 01a1e0123..2517514ba 100644 --- a/src/libsystemd-bus/bus-message.h +++ b/src/libsystemd-bus/bus-message.h @@ -89,14 +89,12 @@ struct sd_bus_message { bool uid_valid:1; bool gid_valid:1; bool free_header:1; - bool free_fields:1; bool free_kdbus:1; bool free_fds:1; bool release_kdbus:1; bool poisoned:1; struct bus_header *header; - void *fields; struct bus_body_part body; struct bus_body_part *body_end; unsigned n_body_parts; @@ -114,7 +112,7 @@ struct sd_bus_message { unsigned n_containers; struct iovec *iovec; - struct iovec iovec_fixed[3]; + struct iovec iovec_fixed[2]; unsigned n_iovec; struct kdbus_msg *kdbus; @@ -178,6 +176,16 @@ static inline uint32_t BUS_MESSAGE_SIZE(sd_bus_message *m) { BUS_MESSAGE_BODY_SIZE(m); } +static inline uint32_t BUS_MESSAGE_BODY_BEGIN(sd_bus_message *m) { + return + sizeof(struct bus_header) + + ALIGN8(BUS_MESSAGE_FIELDS_SIZE(m)); +} + +static inline void* BUS_MESSAGE_FIELDS(sd_bus_message *m) { + return (uint8_t*) m->header + sizeof(struct bus_header); +} + static inline void bus_message_unrefp(sd_bus_message **m) { sd_bus_message_unref(*m); } @@ -214,7 +222,8 @@ int bus_message_append_ap(sd_bus_message *m, const char *types, va_list ap); int bus_message_parse_fields(sd_bus_message *m); -int bus_header_size(struct bus_header *h, size_t *sum); +bool bus_header_is_complete(struct bus_header *h, size_t size); +int bus_header_message_size(struct bus_header *h, size_t *sum); struct bus_body_part *message_append_part(sd_bus_message *m); diff --git a/src/libsystemd-bus/bus-socket.c b/src/libsystemd-bus/bus-socket.c index f43b7da05..4635da4b9 100644 --- a/src/libsystemd-bus/bus-socket.c +++ b/src/libsystemd-bus/bus-socket.c @@ -83,7 +83,7 @@ static int bus_message_setup_iovec(sd_bus_message *m) { assert(!m->iovec); - n = 1 + !!m->fields + m->n_body_parts; + n = 1 + m->n_body_parts; if (n < ELEMENTSOF(m->iovec_fixed)) m->iovec = m->iovec_fixed; else { @@ -92,16 +92,10 @@ static int bus_message_setup_iovec(sd_bus_message *m) { return -ENOMEM; } - r = append_iovec(m, m->header, sizeof(*m->header)); + r = append_iovec(m, m->header, BUS_MESSAGE_BODY_BEGIN(m)); if (r < 0) return r; - if (m->fields) { - r = append_iovec(m, m->fields, ALIGN8(m->header->fields_size)); - if (r < 0) - return r; - } - MESSAGE_FOREACH_PART(part, i, m) { r = append_iovec(m, part->data, part->size); if (r < 0)