fdo-mirrors/dbus
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/dbus/dbus.git synced 2025-12-26 13:20:08 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

Don't cast user-supplied pointers to DBusBasicValue *

Browse source 
If we cast a user-supplied pointer to DBusBasicValue *, that's like
promising that the pointer is to an object at least as strictly aligned
as a DBusBasicValue. In practice, it often isn't: for example, when
we marshal a dbus_uint32_t, a pointer to it is only guaranteed to be
32-bit-aligned, whereas DBusBasicValue typically requires 64-bit
alignment.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie 2018-12-06 13:49:56 +00:00
parent d9de6ab213
commit 26d5d97d5b
6 changed files with 119 additions and 62 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

139
dbus/dbus-marshal-basic.c
View file

@ -37,11 +37,15 @@
#if defined(__GNUC__) && (__GNUC__ >= 4)
# define _DBUS_ASSERT_ALIGNMENT(type, op, val) \
_DBUS_STATIC_ASSERT (__extension__ __alignof__ (type) op val)
# define _DBUS_ASSERT_CMP_ALIGNMENT(left, op, right) \
_DBUS_STATIC_ASSERT (__extension__ __alignof__ (left) op __extension__ __alignof__ (right))
#else
/* not gcc, so probably no alignof operator: just use a no-op statement
* that's valid in the same contexts */
# define _DBUS_ASSERT_ALIGNMENT(type, op, val) \
_DBUS_STATIC_ASSERT (TRUE)
# define _DBUS_ASSERT_CMP_ALIGNMENT(left, op, right) \
_DBUS_STATIC_ASSERT (TRUE)
#endif
/* True by definition, but just for completeness... */
@ -52,21 +56,30 @@ _DBUS_STATIC_ASSERT (sizeof (dbus_int16_t) == 2);
_DBUS_ASSERT_ALIGNMENT (dbus_int16_t, <=, 2);
_DBUS_STATIC_ASSERT (sizeof (dbus_uint16_t) == 2);
_DBUS_ASSERT_ALIGNMENT (dbus_uint16_t, <=, 2);
_DBUS_ASSERT_CMP_ALIGNMENT (dbus_uint16_t, ==, dbus_int16_t);
_DBUS_STATIC_ASSERT (sizeof (dbus_int32_t) == 4);
_DBUS_ASSERT_ALIGNMENT (dbus_int32_t, <=, 4);
_DBUS_STATIC_ASSERT (sizeof (dbus_uint32_t) == 4);
_DBUS_ASSERT_ALIGNMENT (dbus_uint32_t, <=, 4);
_DBUS_ASSERT_CMP_ALIGNMENT (dbus_uint32_t, ==, dbus_int32_t);
_DBUS_STATIC_ASSERT (sizeof (dbus_bool_t) == 4);
_DBUS_ASSERT_ALIGNMENT (dbus_bool_t, <=, 4);
_DBUS_ASSERT_CMP_ALIGNMENT (dbus_uint32_t, ==, dbus_bool_t);
_DBUS_STATIC_ASSERT (sizeof (double) == 8);
_DBUS_ASSERT_ALIGNMENT (double, <=, 8);
/* Doubles might sometimes be more strictly aligned than int64, but we
* assume they are no less strictly aligned. This means every (double *)
* has enough alignment to be treated as though it was a
* (dbus_uint64_t *). */
_DBUS_ASSERT_CMP_ALIGNMENT (dbus_uint64_t, <=, double);
_DBUS_STATIC_ASSERT (sizeof (dbus_int64_t) == 8);
_DBUS_ASSERT_ALIGNMENT (dbus_int64_t, <=, 8);
_DBUS_STATIC_ASSERT (sizeof (dbus_uint64_t) == 8);
_DBUS_ASSERT_ALIGNMENT (dbus_uint64_t, <=, 8);
_DBUS_ASSERT_CMP_ALIGNMENT (dbus_uint64_t, ==, dbus_int64_t);
_DBUS_STATIC_ASSERT (sizeof (DBusBasicValue) >= 8);
/* The alignment of a DBusBasicValue might conceivably be > 8 because of the
@ -117,16 +130,16 @@ pack_4_octets (dbus_uint32_t value,
}
static void
pack_8_octets (DBusBasicValue value,
pack_8_octets (dbus_uint64_t value,
int byte_order,
unsigned char *data)
{
_dbus_assert (_DBUS_ALIGN_ADDRESS (data, 8) == data);
if ((byte_order) == DBUS_LITTLE_ENDIAN)
*((dbus_uint64_t*)(data)) = DBUS_UINT64_TO_LE (value.u64);
*((dbus_uint64_t*)(data)) = DBUS_UINT64_TO_LE (value);
else
*((dbus_uint64_t*)(data)) = DBUS_UINT64_TO_BE (value.u64);
*((dbus_uint64_t*)(data)) = DBUS_UINT64_TO_BE (value);
}
/**
@ -145,12 +158,12 @@ _dbus_pack_uint32 (dbus_uint32_t value,
}
static void
swap_8_octets (DBusBasicValue *value,
swap_8_octets (dbus_uint64_t *value,
int byte_order)
{
if (byte_order != DBUS_COMPILER_BYTE_ORDER)
{
value->u64 = DBUS_UINT64_SWAP_LE_BE (value->u64);
*value = DBUS_UINT64_SWAP_LE_BE (*value);
}
}
@ -231,7 +244,7 @@ set_4_octets (DBusString *str,
static void
set_8_octets (DBusString *str,
int offset,
DBusBasicValue value,
dbus_uint64_t value,
int byte_order)
{
char *data;
@ -380,14 +393,24 @@ _dbus_marshal_set_basic (DBusString *str,
int *old_end_pos,
int *new_end_pos)
{
const DBusBasicValue *vp;
vp = value;
/* Static assertions near the top of this file assert that signed and
* unsigned 16- and 32-bit quantities have the same alignment, and that
* doubles have alignment at least as strict as unsigned int64, so we
* don't have to distinguish further: every (double *)
* has strong enough alignment to be treated as though it was a
* (dbus_uint64_t *). Going via a (void *) means the compiler should
* know that pointers can alias each other. */
const unsigned char *u8_p;
const dbus_uint16_t *u16_p;
const dbus_uint32_t *u32_p;
const dbus_uint64_t *u64_p;
const char * const *string_p;
switch (type)
{
case DBUS_TYPE_BYTE:
_dbus_string_set_byte (str, pos, vp->byt);
u8_p = value;
_dbus_string_set_byte (str, pos, *u8_p);
if (old_end_pos)
*old_end_pos = pos + 1;
if (new_end_pos)
@ -396,8 +419,9 @@ _dbus_marshal_set_basic (DBusString *str,
break;
case DBUS_TYPE_INT16:
case DBUS_TYPE_UINT16:
u16_p = value;
pos = _DBUS_ALIGN_VALUE (pos, 2);
set_2_octets (str, pos, vp->u16, byte_order);
set_2_octets (str, pos, *u16_p, byte_order);
if (old_end_pos)
*old_end_pos = pos + 2;
if (new_end_pos)
@ -408,8 +432,9 @@ _dbus_marshal_set_basic (DBusString *str,
case DBUS_TYPE_INT32:
case DBUS_TYPE_UINT32:
case DBUS_TYPE_UNIX_FD:
u32_p = value;
pos = _DBUS_ALIGN_VALUE (pos, 4);
set_4_octets (str, pos, vp->u32, byte_order);
set_4_octets (str, pos, *u32_p, byte_order);
if (old_end_pos)
*old_end_pos = pos + 4;
if (new_end_pos)
@ -419,8 +444,9 @@ _dbus_marshal_set_basic (DBusString *str,
case DBUS_TYPE_INT64:
case DBUS_TYPE_UINT64:
case DBUS_TYPE_DOUBLE:
u64_p = value;
pos = _DBUS_ALIGN_VALUE (pos, 8);
set_8_octets (str, pos, *vp, byte_order);
set_8_octets (str, pos, *u64_p, byte_order);
if (old_end_pos)
*old_end_pos = pos + 8;
if (new_end_pos)
@ -429,14 +455,16 @@ _dbus_marshal_set_basic (DBusString *str,
break;
case DBUS_TYPE_STRING:
case DBUS_TYPE_OBJECT_PATH:
string_p = value;
pos = _DBUS_ALIGN_VALUE (pos, 4);
_dbus_assert (vp->str != NULL);
return set_string (str, pos, vp->str, byte_order,
_dbus_assert (*string_p != NULL);
return set_string (str, pos, *string_p, byte_order,
old_end_pos, new_end_pos);
break;
case DBUS_TYPE_SIGNATURE:
_dbus_assert (vp->str != NULL);
return set_signature (str, pos, vp->str, byte_order,
string_p = value;
_dbus_assert (*string_p != NULL);
return set_signature (str, pos, *string_p, byte_order,
old_end_pos, new_end_pos);
break;
default:
@ -655,7 +683,7 @@ marshal_4_octets (DBusString *str,
static dbus_bool_t
marshal_8_octets (DBusString *str,
int insert_at,
DBusBasicValue value,
dbus_uint64_t value,
int byte_order,
int *pos_after)
{
@ -803,16 +831,26 @@ _dbus_marshal_write_basic (DBusString *str,
int byte_order,
int *pos_after)
{
const DBusBasicValue *vp;
/* Static assertions near the top of this file assert that signed and
* unsigned 16- and 32-bit quantities have the same alignment, and that
* doubles have alignment at least as strict as unsigned int64, so we
* don't have to distinguish further: every (double *)
* has strong enough alignment to be treated as though it was a
* (dbus_uint64_t *). Going via a (void *) means the compiler should
* know that pointers can alias each other. */
const unsigned char *u8_p;
const dbus_uint16_t *u16_p;
const dbus_uint32_t *u32_p;
const dbus_uint64_t *u64_p;
const char * const *string_p;
_dbus_assert (dbus_type_is_basic (type));
vp = value;
switch (type)
{
case DBUS_TYPE_BYTE:
if (!_dbus_string_insert_byte (str, insert_at, vp->byt))
u8_p = value;
if (!_dbus_string_insert_byte (str, insert_at, *u8_p))
return FALSE;
if (pos_after)
*pos_after = insert_at + 1;
@ -820,33 +858,39 @@ _dbus_marshal_write_basic (DBusString *str,
break;
case DBUS_TYPE_INT16:
case DBUS_TYPE_UINT16:
return marshal_2_octets (str, insert_at, vp->u16,
u16_p = value;
return marshal_2_octets (str, insert_at, *u16_p,
byte_order, pos_after);
break;
case DBUS_TYPE_BOOLEAN:
return marshal_4_octets (str, insert_at, vp->u32 != FALSE,
u32_p = value;
return marshal_4_octets (str, insert_at, (*u32_p != FALSE),
byte_order, pos_after);
break;
case DBUS_TYPE_INT32:
case DBUS_TYPE_UINT32:
case DBUS_TYPE_UNIX_FD:
return marshal_4_octets (str, insert_at, vp->u32,
u32_p = value;
return marshal_4_octets (str, insert_at, *u32_p,
byte_order, pos_after);
break;
case DBUS_TYPE_INT64:
case DBUS_TYPE_UINT64:
case DBUS_TYPE_DOUBLE:
return marshal_8_octets (str, insert_at, *vp, byte_order, pos_after);
u64_p = value;
return marshal_8_octets (str, insert_at, *u64_p, byte_order, pos_after);
break;
case DBUS_TYPE_STRING:
case DBUS_TYPE_OBJECT_PATH:
_dbus_assert (vp->str != NULL);
return marshal_string (str, insert_at, vp->str, byte_order, pos_after);
string_p = value;
_dbus_assert (*string_p != NULL);
return marshal_string (str, insert_at, *string_p, byte_order, pos_after);
break;
case DBUS_TYPE_SIGNATURE:
_dbus_assert (vp->str != NULL);
return marshal_signature (str, insert_at, vp->str, pos_after);
string_p = value;
_dbus_assert (*string_p != NULL);
return marshal_signature (str, insert_at, *string_p, pos_after);
break;
default:
_dbus_assert_not_reached ("not a basic type");
@ -955,7 +999,7 @@ swap_array (DBusString *str,
static dbus_bool_t
marshal_fixed_multi (DBusString *str,
int insert_at,
const DBusBasicValue *value,
const void *value,
int n_elements,
int byte_order,
int alignment,
@ -1030,8 +1074,18 @@ _dbus_marshal_write_fixed_multi (DBusString *str,
int byte_order,
int *pos_after)
{
const void* vp = *(const DBusBasicValue**)value;
/* Static assertions near the top of this file assert that signed and
* unsigned 16- and 32-bit quantities have the same alignment, and that
* doubles have alignment at least as strict as unsigned int64, so we
* don't have to distinguish further: every (double *)
* has strong enough alignment to be treated as though it was a
* (dbus_uint64_t *). Going via a (void *) means the compiler should
* know that pointers can alias each other. */
const unsigned char * const *u8_pp;
const dbus_uint16_t * const *u16_pp;
const dbus_uint32_t * const *u32_pp;
const dbus_uint64_t * const *u64_pp;
_dbus_assert (dbus_type_is_fixed (element_type));
_dbus_assert (n_elements >= 0);
@ -1043,21 +1097,25 @@ _dbus_marshal_write_fixed_multi (DBusString *str,
switch (element_type)
{
case DBUS_TYPE_BYTE:
return marshal_1_octets_array (str, insert_at, vp, n_elements, byte_order, pos_after);
u8_pp = value;
return marshal_1_octets_array (str, insert_at, *u8_pp, n_elements, byte_order, pos_after);
break;
case DBUS_TYPE_INT16:
case DBUS_TYPE_UINT16:
return marshal_fixed_multi (str, insert_at, vp, n_elements, byte_order, 2, pos_after);
u16_pp = value;
return marshal_fixed_multi (str, insert_at, *u16_pp, n_elements, byte_order, 2, pos_after);
case DBUS_TYPE_BOOLEAN:
case DBUS_TYPE_INT32:
case DBUS_TYPE_UINT32:
case DBUS_TYPE_UNIX_FD:
return marshal_fixed_multi (str, insert_at, vp, n_elements, byte_order, 4, pos_after);
u32_pp = value;
return marshal_fixed_multi (str, insert_at, *u32_pp, n_elements, byte_order, 4, pos_after);
break;
case DBUS_TYPE_INT64:
case DBUS_TYPE_UINT64:
case DBUS_TYPE_DOUBLE:
return marshal_fixed_multi (str, insert_at, vp, n_elements, byte_order, 8, pos_after);
u64_pp = value;
return marshal_fixed_multi (str, insert_at, *u64_pp, n_elements, byte_order, 8, pos_after);
break;
default:
@ -1465,7 +1523,7 @@ void
_dbus_marshal_read_fixed_multi (const DBusString *str,
int pos,
int element_type,
void *value,
const void **value,
int n_elements,
int byte_order,
int *new_pos)
@ -1487,7 +1545,7 @@ _dbus_marshal_read_fixed_multi (const DBusString *str,
array_len = n_elements * alignment;
*(const DBusBasicValue**) value = (void*) _dbus_string_get_const_data_len (str, pos, array_len);
*value = _dbus_string_get_const_data_len (str, pos, array_len);
if (new_pos)
*new_pos = pos + array_len;
}
@ -1573,7 +1631,8 @@ swap_test_array (void *array,
int next; \
alignment = _dbus_type_get_alignment (DBUS_TYPE_##typename); \
v_UINT32 = _dbus_marshal_read_uint32 (&str, dump_pos, byte_order, &next); \
_dbus_marshal_read_fixed_multi (&str, next, DBUS_TYPE_##typename, &v_ARRAY_##typename, \
_dbus_marshal_read_fixed_multi (&str, next, DBUS_TYPE_##typename, \
(const void **) &v_ARRAY_##typename, \
v_UINT32/alignment, \
byte_order, NULL); \
swap_test_array (v_ARRAY_##typename, v_UINT32, \

2
dbus/dbus-marshal-basic.h
View file

@ -191,7 +191,7 @@ void _dbus_marshal_read_basic (const DBusString *str,
void _dbus_marshal_read_fixed_multi (const DBusString *str,
int pos,
int element_type,
void *value,
const void **value,
int n_elements,
int byte_order,
int *new_pos);

4
dbus/dbus-marshal-recursive-util.c
View file

@ -2168,7 +2168,7 @@ uint16_read_multi (TestTypeNode *node,
check_expected_type (reader, node->klass->typecode);
_dbus_type_reader_read_fixed_multi (reader,
&values,
(const void **) &values,
&n_elements);
if (n_elements != count)
@ -2294,7 +2294,7 @@ uint32_read_multi (TestTypeNode *node,
check_expected_type (reader, node->klass->typecode);
_dbus_type_reader_read_fixed_multi (reader,
&values,
(const void **) &values,
&n_elements);
if (n_elements != count)

11
dbus/dbus-marshal-recursive.c
View file

@ -922,7 +922,7 @@ _dbus_type_reader_get_array_length (const DBusTypeReader *reader)
*/
void
_dbus_type_reader_read_fixed_multi (const DBusTypeReader *reader,
void *value,
const void **value,
int *n_elements)
{
int element_type;
@ -956,12 +956,11 @@ _dbus_type_reader_read_fixed_multi (const DBusTypeReader *reader,
_dbus_assert (remaining_len <= total_len);
if (remaining_len == 0)
*(const DBusBasicValue**) value = NULL;
*value = NULL;
else
*(const DBusBasicValue**) value =
(void*) _dbus_string_get_const_data_len (reader->value_str,
reader->value_pos,
remaining_len);
*value = _dbus_string_get_const_data_len (reader->value_str,
reader->value_pos,
remaining_len);
*n_elements = remaining_len / alignment;
_dbus_assert ((remaining_len % alignment) == 0);

2
dbus/dbus-marshal-recursive.h
View file

@ -119,7 +119,7 @@ void _dbus_type_reader_read_basic (const DBusTypeReader *
int _dbus_type_reader_get_array_length (const DBusTypeReader *reader);
DBUS_PRIVATE_EXPORT
void _dbus_type_reader_read_fixed_multi (const DBusTypeReader *reader,
void *value,
const void **value,
int *n_elements);
void _dbus_type_reader_read_raw (const DBusTypeReader *reader,
const unsigned char **value_location);

23
dbus/dbus-message.c
View file

@ -893,9 +893,9 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
}
else if (dbus_type_is_basic (spec_type))
{
DBusBasicValue *ptr;
void *ptr;
ptr = va_arg (var_args, DBusBasicValue*);
ptr = va_arg (var_args, void *);
_dbus_assert (ptr != NULL);
@ -906,7 +906,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
{
int element_type;
int spec_element_type;
const DBusBasicValue **ptr;
const void **ptr;
int *n_elements_p;
DBusTypeReader array;
@ -928,7 +928,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
if (dbus_type_is_fixed (spec_element_type) &&
element_type != DBUS_TYPE_UNIX_FD)
{
ptr = va_arg (var_args, const DBusBasicValue**);
ptr = va_arg (var_args, const void **);
n_elements_p = va_arg (var_args, int*);
_dbus_assert (ptr != NULL);
@ -936,8 +936,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
_dbus_type_reader_recurse (&real->u.reader, &array);
_dbus_type_reader_read_fixed_multi (&array,
(void *) ptr, n_elements_p);
_dbus_type_reader_read_fixed_multi (&array, ptr, n_elements_p);
}
else if (_DBUS_TYPE_IS_STRINGLIKE (spec_element_type))
{
@ -1060,7 +1059,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
else if (dbus_type_is_basic (spec_type))
{
/* move the index forward */
va_arg (copy_args, DBusBasicValue *);
va_arg (copy_args, const void *);
}
else if (spec_type == DBUS_TYPE_ARRAY)
{
@ -1070,7 +1069,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
if (dbus_type_is_fixed (spec_element_type))
{
/* move the index forward */
va_arg (copy_args, const DBusBasicValue **);
va_arg (copy_args, const void **);
va_arg (copy_args, int *);
}
else if (_DBUS_TYPE_IS_STRINGLIKE (spec_element_type))
@ -1872,8 +1871,8 @@ dbus_message_append_args_valist (DBusMessage *message,
{
if (dbus_type_is_basic (type))
{
const DBusBasicValue *value;
value = va_arg (var_args, const DBusBasicValue*);
const void *value;
value = va_arg (var_args, const void *);
if (!dbus_message_iter_append_basic (&iter,
type,
@ -1899,10 +1898,10 @@ dbus_message_append_args_valist (DBusMessage *message,
if (dbus_type_is_fixed (element_type) &&
element_type != DBUS_TYPE_UNIX_FD)
{
const DBusBasicValue **value;
const void **value;
int n_elements;
value = va_arg (var_args, const DBusBasicValue**);
value = va_arg (var_args, const void **);
n_elements = va_arg (var_args, int);
if (!dbus_message_iter_append_fixed_array (&array,