From dac167f036465e9d7cca10c52d8345773d2e6c3f Mon Sep 17 00:00:00 2001 From: Flavio Cruz Date: Sun, 19 Feb 2023 18:06:51 -0500 Subject: Support alignment requirements for a 64 bit kernel. We introduce both a user alignment and a kernel alignment. These are separate requirements since for 64 bit with a 32 bit kernel we need to ensure the kernel can consume messages that are 8-byte aligned. This change removes any possibility of undefined behavior and also allows the kernel to support 64 bit RPCs for the userland. A lot of the code that performs alignment was simplified under the assumption that the message headers are well aligned. To enforce that going forward, a few static assertions were added. Message-Id: --- include/mach/message.h | 34 ++++++++---- ipc/ipc_kmsg.c | 144 +++++++++++++++++++------------------------------ x86_64/copy_user.c | 36 ++++++++----- 3 files changed, 101 insertions(+), 113 deletions(-) diff --git a/include/mach/message.h b/include/mach/message.h index eb3b34c0..22a17b03 100644 --- a/include/mach/message.h +++ b/include/mach/message.h @@ -334,19 +334,33 @@ typedef integer_t mach_msg_option_t; #define MACH_SEND_ALWAYS 0x00010000 /* internal use only */ -/* This is the alignment of msg descriptors and the actual data. +#ifdef KERNEL +/* This is the alignment of msg descriptors and the actual data + * for both in kernel messages and user land messages. * - * On x86 it is made equal to the default structure alignment on - * 32-bit, so we can easily maintain compatibility with 32-bit user - * space on a 64-bit kernel. Other architectures might have different - * needs, so this value might change in the future for differents - * architectures. + * We have two types of alignment because for specific configurations + * (in particular a 64 bit kernel with 32 bit userland) we transform + * 4-byte aligned user messages into 8-byte aligned messages (and vice-versa) + * so that kernel messages are correctly aligned. */ -#define MACH_MSG_ALIGNMENT 4 +#define MACH_MSG_KERNEL_ALIGNMENT sizeof(uintptr_t) +#ifdef __x86_64__ +#ifdef USER32 +#define MACH_MSG_USER_ALIGNMENT 4 +#else +#define MACH_MSG_USER_ALIGNMENT 8 +#endif +#else +#define MACH_MSG_USER_ALIGNMENT 4 +#endif -#define mach_msg_is_misaligned(x) ( ((vm_offset_t)(x)) & (MACH_MSG_ALIGNMENT-1) ) -#define mach_msg_align(x) \ - ( ( ((vm_offset_t)(x)) + (MACH_MSG_ALIGNMENT-1) ) & ~(MACH_MSG_ALIGNMENT-1) ) +#define mach_msg_align(x, alignment) \ + ( ( ((vm_offset_t)(x)) + ((alignment)-1) ) & ~((alignment)-1) ) +#define mach_msg_user_align(x) mach_msg_align(x, MACH_MSG_USER_ALIGNMENT) +#define mach_msg_kernel_align(x) mach_msg_align(x, MACH_MSG_KERNEL_ALIGNMENT) +#define mach_msg_user_is_misaligned(x) ((x) & ((MACH_MSG_USER_ALIGNMENT)-1)) +#define mach_msg_kernel_is_misaligned(x) ((x) & ((MACH_MSG_KERNEL_ALIGNMENT)-1)) +#endif /* KERNEL */ /* * Much code assumes that mach_msg_return_t == kern_return_t. diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c index dac4f5dc..1988da45 100644 --- a/ipc/ipc_kmsg.c +++ b/ipc/ipc_kmsg.c @@ -229,28 +229,23 @@ ipc_kmsg_clean_body( type = (mach_msg_type_long_t *) saddr; is_inline = ((mach_msg_type_t*)type)->msgt_inline; if (((mach_msg_type_t*)type)->msgt_longform) { - /* This must be aligned */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - (mach_msg_is_misaligned(type))) { - saddr = mach_msg_align(saddr); - continue; - } name = type->msgtl_name; size = type->msgtl_size; number = type->msgtl_number; saddr += sizeof(mach_msg_type_long_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_long_t))) { + saddr = mach_msg_kernel_align(saddr); + } } else { name = ((mach_msg_type_t*)type)->msgt_name; size = ((mach_msg_type_t*)type)->msgt_size; number = ((mach_msg_type_t*)type)->msgt_number; saddr += sizeof(mach_msg_type_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_t))) { + saddr = mach_msg_kernel_align(saddr); + } } - /* padding (ptrs and ports) ? */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - ((size >> 3) == sizeof(natural_t))) - saddr = mach_msg_align(saddr); - /* calculate length of data in bytes, rounding up */ length = ((number * size) + 7) >> 3; @@ -283,9 +278,7 @@ ipc_kmsg_clean_body( } if (is_inline) { - /* inline data sizes round up to int boundaries */ - - saddr += (length + 3) &~ 3; + saddr += length; } else { vm_offset_t data = * (vm_offset_t *) saddr; @@ -300,6 +293,7 @@ ipc_kmsg_clean_body( saddr += sizeof(vm_offset_t); } + saddr = mach_msg_kernel_align(saddr); } } @@ -387,31 +381,26 @@ ipc_kmsg_clean_partial( boolean_t is_inline, is_port; vm_size_t length; -xxx: type = (mach_msg_type_long_t *) eaddr; + type = (mach_msg_type_long_t *) eaddr; is_inline = ((mach_msg_type_t*)type)->msgt_inline; if (((mach_msg_type_t*)type)->msgt_longform) { - /* This must be aligned */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - (mach_msg_is_misaligned(type))) { - eaddr = mach_msg_align(eaddr); - goto xxx; - } name = type->msgtl_name; size = type->msgtl_size; rnumber = type->msgtl_number; eaddr += sizeof(mach_msg_type_long_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_long_t))) { + eaddr = mach_msg_kernel_align(eaddr); + } } else { name = ((mach_msg_type_t*)type)->msgt_name; size = ((mach_msg_type_t*)type)->msgt_size; rnumber = ((mach_msg_type_t*)type)->msgt_number; eaddr += sizeof(mach_msg_type_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_t))) { + eaddr = mach_msg_kernel_align(eaddr); + } } - /* padding (ptrs and ports) ? */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - ((size >> 3) == sizeof(natural_t))) - eaddr = mach_msg_align(eaddr); - /* calculate length of data in bytes, rounding up */ length = ((rnumber * size) + 7) >> 3; @@ -501,7 +490,7 @@ ipc_kmsg_get( { ipc_kmsg_t kmsg; - if ((size < sizeof(mach_msg_user_header_t)) || (size & 3)) + if ((size < sizeof(mach_msg_user_header_t)) || mach_msg_user_is_misaligned(size)) return MACH_SEND_MSG_TOO_SMALL; if (size <= IKM_SAVED_MSG_SIZE) { @@ -553,7 +542,7 @@ ipc_kmsg_get_from_kernel( ipc_kmsg_t kmsg; assert(size >= sizeof(mach_msg_header_t)); - assert((size & 3) == 0); + assert(!mach_msg_kernel_is_misaligned(size)); kmsg = ikm_alloc(size); if (kmsg == IKM_NULL) @@ -1307,6 +1296,10 @@ ipc_kmsg_copyin_body( saddr = (vm_offset_t) (&kmsg->ikm_header + 1); eaddr = (vm_offset_t) &kmsg->ikm_header + kmsg->ikm_header.msgh_size; + // We make assumptions about the alignment of the header. + _Static_assert(!mach_msg_kernel_is_misaligned(sizeof(mach_msg_header_t)), + "mach_msg_header_t needs to be MACH_MSG_KERNEL_ALIGNMENT aligned."); + while (saddr < eaddr) { vm_offset_t taddr = saddr; mach_msg_type_long_t *type; @@ -1330,21 +1323,21 @@ ipc_kmsg_copyin_body( is_inline = ((mach_msg_type_t*)type)->msgt_inline; dealloc = ((mach_msg_type_t*)type)->msgt_deallocate; if (longform) { - /* This must be aligned */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - (mach_msg_is_misaligned(type))) { - saddr = mach_msg_align(saddr); - continue; - } name = type->msgtl_name; size = type->msgtl_size; number = type->msgtl_number; saddr += sizeof(mach_msg_type_long_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_long_t))) { + saddr = mach_msg_kernel_align(saddr); + } } else { name = ((mach_msg_type_t*)type)->msgt_name; size = ((mach_msg_type_t*)type)->msgt_size; number = ((mach_msg_type_t*)type)->msgt_number; saddr += sizeof(mach_msg_type_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_t))) { + saddr = mach_msg_kernel_align(saddr); + } } is_port = MACH_MSG_TYPE_PORT_ANY(name); @@ -1359,21 +1352,13 @@ ipc_kmsg_copyin_body( return MACH_SEND_INVALID_TYPE; } - /* padding (ptrs and ports) ? */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - ((size >> 3) == sizeof(natural_t))) - saddr = mach_msg_align(saddr); - /* calculate length of data in bytes, rounding up */ length = (((uint64_t) number * size) + 7) >> 3; if (is_inline) { - vm_size_t amount; - - /* inline data sizes round up to int boundaries */ + vm_size_t amount = length; - amount = (length + 3) &~ 3; if ((eaddr - saddr) < amount) { ipc_kmsg_clean_partial(kmsg, taddr, FALSE, 0); return MACH_SEND_MSG_TOO_SMALL; @@ -1384,9 +1369,6 @@ ipc_kmsg_copyin_body( } else { vm_offset_t addr; - if (MACH_MSG_ALIGNMENT > sizeof(mach_msg_type_t)) - saddr = mach_msg_align(saddr); - if ((eaddr - saddr) < sizeof(vm_offset_t)) { ipc_kmsg_clean_partial(kmsg, taddr, FALSE, 0); return MACH_SEND_MSG_TOO_SMALL; @@ -1491,6 +1473,7 @@ ipc_kmsg_copyin_body( complex = TRUE; } + saddr = mach_msg_kernel_align(saddr); } if (!complex) @@ -1613,28 +1596,23 @@ ipc_kmsg_copyin_from_kernel(ipc_kmsg_t kmsg) longform = ((mach_msg_type_t*)type)->msgt_longform; /* type->msgtl_header.msgt_deallocate not used */ if (longform) { - /* This must be aligned */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - (mach_msg_is_misaligned(type))) { - saddr = mach_msg_align(saddr); - continue; - } name = type->msgtl_name; size = type->msgtl_size; number = type->msgtl_number; saddr += sizeof(mach_msg_type_long_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_long_t))) { + saddr = mach_msg_kernel_align(saddr); + } } else { name = ((mach_msg_type_t*)type)->msgt_name; size = ((mach_msg_type_t*)type)->msgt_size; number = ((mach_msg_type_t*)type)->msgt_number; saddr += sizeof(mach_msg_type_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_t))) { + saddr = mach_msg_kernel_align(saddr); + } } - /* padding (ptrs and ports) ? */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - ((size >> 3) == sizeof(natural_t))) - saddr = mach_msg_align(saddr); - /* calculate length of data in bytes, rounding up */ length = ((number * size) + 7) >> 3; @@ -1642,10 +1620,8 @@ ipc_kmsg_copyin_from_kernel(ipc_kmsg_t kmsg) is_port = MACH_MSG_TYPE_PORT_ANY(name); if (is_inline) { - /* inline data sizes round up to int boundaries */ - data = saddr; - saddr += (length + 3) &~ 3; + saddr += length; } else { /* * The sender should supply ready-made memory @@ -1682,6 +1658,7 @@ ipc_kmsg_copyin_from_kernel(ipc_kmsg_t kmsg) MACH_MSGH_BITS_CIRCULAR; } } + saddr = mach_msg_kernel_align(saddr); } } @@ -2386,28 +2363,23 @@ ipc_kmsg_copyout_body( is_inline = ((mach_msg_type_t*)type)->msgt_inline; longform = ((mach_msg_type_t*)type)->msgt_longform; if (longform) { - /* This must be aligned */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - (mach_msg_is_misaligned(type))) { - saddr = mach_msg_align(saddr); - continue; - } name = type->msgtl_name; size = type->msgtl_size; number = type->msgtl_number; saddr += sizeof(mach_msg_type_long_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_long_t))) { + saddr = mach_msg_kernel_align(saddr); + } } else { name = ((mach_msg_type_t*)type)->msgt_name; size = ((mach_msg_type_t*)type)->msgt_size; number = ((mach_msg_type_t*)type)->msgt_number; saddr += sizeof(mach_msg_type_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_t))) { + saddr = mach_msg_kernel_align(saddr); + } } - /* padding (ptrs and ports) ? */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - ((size >> 3) == sizeof(natural_t))) - saddr = mach_msg_align(saddr); - /* calculate length of data in bytes, rounding up */ length = (((uint64_t) number * size) + 7) >> 3; @@ -2442,16 +2414,11 @@ ipc_kmsg_copyout_body( } if (is_inline) { - /* inline data sizes round up to int boundaries */ - ((mach_msg_type_t*)type)->msgt_deallocate = FALSE; - saddr += (length + 3) &~ 3; + saddr += length; } else { vm_offset_t data; - if (MACH_MSG_ALIGNMENT > sizeof(mach_msg_type_t)) - saddr = mach_msg_align(saddr); - data = * (vm_offset_t *) saddr; /* copyout memory carried in the message */ @@ -2502,6 +2469,9 @@ ipc_kmsg_copyout_body( * (vm_offset_t *) saddr = addr; saddr += sizeof(vm_offset_t); } + + /* Next element is always correctly aligned */ + saddr = mach_msg_kernel_align(saddr); } return mr; @@ -2827,21 +2797,21 @@ ipc_msg_print(mach_msg_header_t *msgh) is_inline = ((mach_msg_type_t*)type)->msgt_inline; dealloc = ((mach_msg_type_t*)type)->msgt_deallocate; if (longform) { - /* This must be aligned */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - (mach_msg_is_misaligned(type))) { - saddr = mach_msg_align(saddr); - continue; - } name = type->msgtl_name; size = type->msgtl_size; number = type->msgtl_number; saddr += sizeof(mach_msg_type_long_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_long_t))) { + saddr = mach_msg_kernel_align(saddr); + } } else { name = ((mach_msg_type_t*)type)->msgt_name; size = ((mach_msg_type_t*)type)->msgt_size; number = ((mach_msg_type_t*)type)->msgt_number; saddr += sizeof(mach_msg_type_t); + if (mach_msg_kernel_is_misaligned(sizeof(mach_msg_type_t))) { + saddr = mach_msg_kernel_align(saddr); + } } db_printf("-- type="); @@ -2872,11 +2842,6 @@ ipc_msg_print(mach_msg_header_t *msgh) return; } - /* padding (ptrs and ports) ? */ - if ((sizeof(natural_t) > sizeof(mach_msg_type_t)) && - ((size >> 3) == sizeof(natural_t))) - saddr = mach_msg_align(saddr); - /* calculate length of data in bytes, rounding up */ length = ((number * size) + 7) >> 3; @@ -2885,7 +2850,7 @@ ipc_msg_print(mach_msg_header_t *msgh) vm_size_t amount; unsigned i, numwords; - /* inline data sizes round up to int boundaries */ + /* round up to int boundaries for printing */ amount = (length + 3) &~ 3; if ((eaddr - saddr) < amount) { db_printf("*** too small\n"); @@ -2910,6 +2875,7 @@ ipc_msg_print(mach_msg_header_t *msgh) db_printf("0x%x\n", * (vm_offset_t *) saddr); saddr += sizeof(vm_offset_t); } + saddr = mach_msg_kernel_align(saddr); } } #endif /* MACH_KDB */ diff --git a/x86_64/copy_user.c b/x86_64/copy_user.c index ae17c368..e429f259 100644 --- a/x86_64/copy_user.c +++ b/x86_64/copy_user.c @@ -136,7 +136,9 @@ size_t msg_usize(const mach_msg_header_t *kmsg) boolean_t is_inline; amount = unpack_msg_type(saddr, &name, &size, &number, &is_inline); saddr += amount; + saddr = mach_msg_kernel_align(saddr); usize += amount; + usize = mach_msg_user_align(usize); if (is_inline) { @@ -150,8 +152,6 @@ size_t msg_usize(const mach_msg_header_t *kmsg) size_t n = descsize_to_bytes(size); saddr += n*number; usize += n*number; - saddr = mach_msg_align(saddr); - usize = mach_msg_align(usize); } } else @@ -160,6 +160,8 @@ size_t msg_usize(const mach_msg_header_t *kmsg) saddr += sizeof(vm_offset_t); usize += sizeof(rpc_vm_offset_t); } + saddr = mach_msg_kernel_align(saddr); + usize = mach_msg_user_align(usize); } } return usize; @@ -168,7 +170,7 @@ size_t msg_usize(const mach_msg_header_t *kmsg) /* * Expand the msg header and, if required, the msg body (ports, pointers) * - * To not make the code too compicated, we use the fact that some fields of + * To not make the code too complicated, we use the fact that some fields of * mach_msg_header have the same size in the kernel and user variant (basically * all fields except ports and addresses) */ @@ -202,6 +204,10 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const size_t usize) ksaddr = (vm_offset_t)(kmsg + 1); usaddr = (vm_offset_t)(umsg + 1); ueaddr = (vm_offset_t)umsg + usize; + + _Static_assert(!mach_msg_user_is_misaligned(sizeof(mach_msg_user_header_t)), + "mach_msg_user_header_t needs to be MACH_MSG_USER_ALIGNMENT aligned."); + if (usize > sizeof(mach_msg_user_header_t)) { /* check we have at least space for an empty descryptor */ @@ -212,8 +218,6 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const size_t usize) mach_msg_type_size_t size; mach_msg_type_number_t number; boolean_t is_inline; - usaddr = mach_msg_align(usaddr); - ksaddr = mach_msg_align(ksaddr); if (copyin_unpack_msg_type(usaddr, ksaddr, &name, &size, &number, &is_inline, &amount)) return 1; @@ -222,7 +226,9 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const size_t usize) // might need to adjust it later depending on the type vm_offset_t ktaddr = ksaddr; usaddr += amount; + usaddr = mach_msg_user_align(usaddr); ksaddr += amount; + ksaddr = mach_msg_kernel_align(ksaddr); if (is_inline) { @@ -249,8 +255,6 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const size_t usize) return 1; usaddr += n*number; ksaddr += n*number; - usaddr = mach_msg_align(usaddr); - ksaddr = mach_msg_align(ksaddr); } } else @@ -264,15 +268,19 @@ int copyinmsg (const void *userbuf, void *kernelbuf, const size_t usize) if (copyin_address((rpc_vm_offset_t*)usaddr, (vm_offset_t*)ksaddr)) return 1; - // advance one pointer + // Advance one pointer. ksaddr += sizeof(vm_offset_t); usaddr += sizeof(rpc_vm_offset_t); } + // Note that we have to align because mach_port_name_t might not align + // with the required user alignment. + usaddr = mach_msg_user_align(usaddr); + ksaddr = mach_msg_kernel_align(ksaddr); } } kmsg->msgh_size = sizeof(mach_msg_header_t) + ksaddr - (vm_offset_t)(kmsg + 1); - kmsg->msgh_size = mach_msg_align(kmsg->msgh_size); + kmsg->msgh_size = kmsg->msgh_size; return 0; } @@ -310,15 +318,15 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, const size_t ksize) mach_msg_type_size_t size; mach_msg_type_number_t number; boolean_t is_inline; - usaddr = mach_msg_align(usaddr); - ksaddr = mach_msg_align(ksaddr); amount = unpack_msg_type(ksaddr, &name, &size, &number, &is_inline); // TODO: optimize and bring here type adjustment?? vm_offset_t utaddr=usaddr, ktaddr=ksaddr; if (copyout((void*)ksaddr, (void*)usaddr, amount)) return 1; usaddr += amount; + usaddr = mach_msg_user_align(usaddr); ksaddr += amount; + ksaddr = mach_msg_kernel_align(ksaddr); if (is_inline) { @@ -343,8 +351,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, const size_t ksize) return 1; usaddr += n*number; ksaddr += n*number; - usaddr = mach_msg_align(usaddr); - ksaddr = mach_msg_align(ksaddr); } } else @@ -363,12 +369,14 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, const size_t ksize) ksaddr += sizeof(vm_offset_t); usaddr += sizeof(rpc_vm_offset_t); } + usaddr = mach_msg_user_align(usaddr); + ksaddr = mach_msg_kernel_align(ksaddr); } } mach_msg_size_t usize; usize = sizeof(mach_msg_user_header_t) + usaddr - (vm_offset_t)(umsg + 1); - usize = mach_msg_align(usize); + usize = usize; if (copyout(&usize, &umsg->msgh_size, sizeof(kmsg->msgh_size))) return 1; -- cgit v1.2.3