Applied, thanks! Flavio Cruz, le lun. 19 févr. 2024 23:58:00 -0500, a ecrit: > This is a follow up to > https://git.savannah.gnu.org/cgit/hurd/gnumach.git/commit/?id=69620634858b2992e1a362e33c95d9a8ee57bce7 > where we made inlined ports 8 bytes long to avoid resizing. > > The last thing that copy{in,out}msg were doing was just updating > msgt_size field since that's required for kernel stub code and implicitly > assumed by IPC code. This was moved into ipc_kmsg_copy{in,out}_body. > > For a 32 bit userland, the code also stops updating > msgt_size for out of line ports, same as the 64 bit userland. > --- > i386/i386/locore.h | 6 +++ > ipc/ipc_kmsg.c | 52 +++++++++++++------ > x86_64/copy_user.c | 123 +++++++++------------------------------------ > 3 files changed, 66 insertions(+), 115 deletions(-) > > diff --git a/i386/i386/locore.h b/i386/i386/locore.h > index 374c8cf..217e6de 100644 > --- a/i386/i386/locore.h > +++ b/i386/i386/locore.h > @@ -50,7 +50,13 @@ extern int discover_x86_cpu_type (void); > extern int copyin (const void *userbuf, void *kernelbuf, size_t cn); > extern int copyinmsg (const void *userbuf, void *kernelbuf, size_t cn, > size_t kn); > extern int copyout (const void *kernelbuf, void *userbuf, size_t cn); > +#ifdef USER32 > extern int copyoutmsg (const void *kernelbuf, void *userbuf, size_t cn); > +#else > +static inline int copyoutmsg (const void *kernelbuf, void *userbuf, size_t > cn) { > + return copyout (kernelbuf, userbuf, cn); > +} > +#endif > > extern int inst_fetch (int eip, int cs); > > diff --git a/ipc/ipc_kmsg.c b/ipc/ipc_kmsg.c > index bd84380..6a7ad3c 100644 > --- a/ipc/ipc_kmsg.c > +++ b/ipc/ipc_kmsg.c > @@ -1321,7 +1321,7 @@ ipc_kmsg_copyin_body( > mach_msg_type_number_t number; > boolean_t is_inline, longform, dealloc, is_port; > vm_offset_t data; > - uint64_t length; > + vm_size_t length; > kern_return_t kr; > > type = (mach_msg_type_long_t *) saddr; > @@ -1355,7 +1355,8 @@ ipc_kmsg_copyin_body( > > is_port = MACH_MSG_TYPE_PORT_ANY(name); > > - if ((is_port && (size != PORT_T_SIZE_IN_BITS)) || > + if ((is_port && !is_inline && (size != > PORT_NAME_T_SIZE_IN_BITS)) || > + (is_port && is_inline && (size != PORT_T_SIZE_IN_BITS)) || > #ifndef __x86_64__ > (longform && ((type->msgtl_header.msgt_name != 0) || > (type->msgtl_header.msgt_size != 0) || > @@ -1396,11 +1397,22 @@ ipc_kmsg_copyin_body( > if (length == 0) > data = 0; > else if (is_port) { > + const vm_size_t user_length = length; > + /* > + * In 64 bit architectures, out of line port > names are > + * represented as an array of mach_port_name_t > which are > + * smaller than mach_port_t. > + */ > + if (sizeof(mach_port_name_t) != > sizeof(mach_port_t)) { > + length = sizeof(mach_port_t) * number; > + type->msgtl_size = sizeof(mach_port_t) > * 8; > + } > + > data = kalloc(length); > if (data == 0) > goto invalid_memory; > > - if (sizeof(mach_port_name_t) != > sizeof(mach_port_t)) > + if (user_length != length) > { > mach_port_name_t *src = > (mach_port_name_t*)addr; > mach_port_t *dst = (mach_port_t*)data; > @@ -1416,7 +1428,7 @@ ipc_kmsg_copyin_body( > goto invalid_memory; > } > if (dealloc && > - (vm_deallocate(map, addr, length) != > KERN_SUCCESS)) { > + (vm_deallocate(map, addr, user_length) != > KERN_SUCCESS)) { > kfree(data, length); > goto invalid_memory; > } > @@ -2372,7 +2384,7 @@ ipc_kmsg_copyout_body( > mach_msg_type_size_t size; > mach_msg_type_number_t number; > boolean_t is_inline, longform, is_port; > - uint64_t length; > + vm_size_t length; > vm_offset_t addr; > > type = (mach_msg_type_long_t *) saddr; > @@ -2406,18 +2418,28 @@ ipc_kmsg_copyout_body( > ipc_object_t *objects; > mach_msg_type_number_t i; > > - if (!is_inline && (length != 0)) { > - /* first allocate memory in the map */ > - uint64_t allocated = length; > + if (!is_inline) { > + if (length != 0) { > + vm_size_t user_length = length; > > - _Static_assert(sizeof(mach_port_name_t) <= > sizeof(mach_port_t), > - "Size of mach_port_t should be > equal or larger than mach_port_name_t."); > - allocated -= (sizeof(mach_port_t) - > sizeof(mach_port_name_t)) * number; > + if (sizeof(mach_port_name_t) != > sizeof(mach_port_t)) { > + user_length = > sizeof(mach_port_name_t) * number; > + } > > - kr = vm_allocate(map, &addr, allocated, TRUE); > - if (kr != KERN_SUCCESS) { > - ipc_kmsg_clean_body(taddr, saddr); > - goto vm_copyout_failure; > + /* first allocate memory in the map */ > + kr = vm_allocate(map, &addr, > user_length, TRUE); > + if (kr != KERN_SUCCESS) { > + ipc_kmsg_clean_body(taddr, > saddr); > + goto vm_copyout_failure; > + } > + } > + > + if (sizeof(mach_port_name_t) != > sizeof(mach_port_t)) { > + /* Out of line ports are always > returned as mach_port_name_t. > + * Note: we have to do this after > ipc_kmsg_clean_body, otherwise > + * the cleanup function will not work > correctly. > + */ > + type->msgtl_size = > sizeof(mach_port_name_t) * 8; > } > } > > diff --git a/x86_64/copy_user.c b/x86_64/copy_user.c > index c6e125d..89acd81 100644 > --- a/x86_64/copy_user.c > +++ b/x86_64/copy_user.c > @@ -49,10 +49,6 @@ typedef struct { > natural_t msgtl_number; > } mach_msg_user_type_long_t; > _Static_assert(sizeof(mach_msg_user_type_long_t) == 12); > -#else > -typedef mach_msg_type_t mach_msg_user_type_t; > -typedef mach_msg_type_long_t mach_msg_user_type_long_t; > -#endif /* USER32 */ > > /* > * Helper to unpack the relevant fields of a msg type; the fields are > different > @@ -87,7 +83,6 @@ static inline void unpack_msg_type(vm_offset_t addr, > } > } > > -#ifdef USER32 > static inline void mach_msg_user_type_to_kernel(const mach_msg_user_type_t > *u, > mach_msg_type_t* k) { > k->msgt_name = u->msgt_name; > @@ -144,53 +139,33 @@ static inline void > mach_msg_kernel_type_to_user_long(const mach_msg_type_long_t > }; > *u = user; > } > -#endif > > static inline int copyin_mach_msg_type(const rpc_vm_offset_t *uaddr, > mach_msg_type_t *kaddr) { > -#ifdef USER32 > mach_msg_user_type_t user; > - int ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_t)); > - if (ret) { > - return ret; > - } > + if (copyin(uaddr, &user, sizeof(mach_msg_user_type_t))) > + return 1; > mach_msg_user_type_to_kernel(&user, kaddr); > return 0; > -#else > - return copyin(uaddr, kaddr, sizeof(mach_msg_type_t)); > -#endif > } > > static inline int copyout_mach_msg_type(const mach_msg_type_t *kaddr, > rpc_vm_offset_t *uaddr) { > -#ifdef USER32 > mach_msg_user_type_t user; > mach_msg_kernel_type_to_user(kaddr, &user); > return copyout(&user, uaddr, sizeof(mach_msg_user_type_t)); > -#else > - return copyout(kaddr, uaddr, sizeof(mach_msg_type_t)); > -#endif > } > > static inline int copyin_mach_msg_type_long(const rpc_vm_offset_t *uaddr, > mach_msg_type_long_t *kaddr) { > -#ifdef USER32 > mach_msg_user_type_long_t user; > - int ret = copyin(uaddr, &user, sizeof(mach_msg_user_type_long_t)); > - if (ret) > - return ret; > + if (copyin(uaddr, &user, sizeof(mach_msg_user_type_long_t))) > + return 1; > mach_msg_user_type_to_kernel_long(&user, kaddr); > return 0; > -#else > - return copyin(uaddr, kaddr, sizeof(mach_msg_type_long_t)); > -#endif > } > > static inline int copyout_mach_msg_type_long(const mach_msg_type_long_t > *kaddr, rpc_vm_offset_t *uaddr) { > -#ifdef USER32 > mach_msg_user_type_long_t user; > mach_msg_kernel_type_to_user_long(kaddr, &user); > return copyout(&user, uaddr, sizeof(mach_msg_user_type_long_t)); > -#else > - return copyout(kaddr, uaddr, sizeof(mach_msg_type_long_t)); > -#endif > } > > /* Optimized version of unpack_msg_type(), including proper copyin() */ > @@ -265,15 +240,8 @@ static inline int copyout_unpack_msg_type(vm_offset_t > kaddr, > mach_msg_type_size_t orig_size = kmtl->msgtl_size; > int ret; > > - if (MACH_MSG_TYPE_PORT_ANY(kmtl->msgtl_name)) { > -#ifdef USER32 > - kmtl->msgtl_size = bytes_to_descsize(sizeof(mach_port_name_t)); > -#else > - /* 64 bit ABI uses mach_port_name_inlined_t for inlined ports. */ > - if (!kmt->msgt_inline) > + if (MACH_MSG_TYPE_PORT_ANY(kmtl->msgtl_name) && kmt->msgt_inline) > kmtl->msgtl_size = bytes_to_descsize(sizeof(mach_port_name_t)); > -#endif > - } > ret = copyout_mach_msg_type_long(kmtl, (void*)uaddr); > kmtl->msgtl_size = orig_size; > if (ret) > @@ -289,21 +257,12 @@ static inline int copyout_unpack_msg_type(vm_offset_t > kaddr, > { > mach_msg_type_size_t orig_size = kmt->msgt_size; > int ret; > - > - if (MACH_MSG_TYPE_PORT_ANY(kmt->msgt_name)) { > -#ifdef USER32 > + if (MACH_MSG_TYPE_PORT_ANY(kmt->msgt_name) && kmt->msgt_inline) > kmt->msgt_size = bytes_to_descsize(sizeof(mach_port_name_t)); > -#else > - /* 64 bit ABI uses mach_port_name_inlined_t for inlined ports. */ > - if (!kmt->msgt_inline) > - kmt->msgt_size = bytes_to_descsize(sizeof(mach_port_name_t)); > -#endif > - } > ret = copyout_mach_msg_type(kmt, (void *)uaddr); > kmt->msgt_size = orig_size; > if (ret) > return 1; > - > *name = kmt->msgt_name; > *size = kmt->msgt_size; > *number = kmt->msgt_number; > @@ -313,7 +272,6 @@ static inline int copyout_unpack_msg_type(vm_offset_t > kaddr, > return 0; > } > > -#ifdef USER32 > /* > * Compute the user-space size of a message still in the kernel when > processing > * messages from 32bit userland. > @@ -372,7 +330,7 @@ size_t msg_usize(const mach_msg_header_t *kmsg) > } > return usize; > } > -#endif /* USER32 */ > +#endif /* USER32 */ > > /* > * Expand the msg header and, if required, the msg body (ports, pointers) > @@ -386,6 +344,9 @@ int copyinmsg (const void *userbuf, void *kernelbuf, > const size_t usize, const s > const mach_msg_user_header_t *umsg = userbuf; > mach_msg_header_t *kmsg = kernelbuf; > > + > _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."); > + > #ifdef USER32 > if (copyin(&umsg->msgh_bits, &kmsg->msgh_bits, sizeof(kmsg->msgh_bits))) > return 1; > @@ -397,23 +358,12 @@ int copyinmsg (const void *userbuf, void *kernelbuf, > const size_t usize, const s > if (copyin(&umsg->msgh_seqno, &kmsg->msgh_seqno, > sizeof(kmsg->msgh_seqno) + sizeof(kmsg->msgh_id))) > return 1; > -#else > - /* The 64 bit interface ensures the header is the same size, so it does > not need any resizing. */ > - _Static_assert(sizeof(mach_msg_header_t) == sizeof(mach_msg_user_header_t), > - "mach_msg_header_t and mach_msg_user_header_t expected to be > of the same size"); > - if (copyin(umsg, kmsg, sizeof(mach_msg_header_t))) > - return 1; > - kmsg->msgh_remote_port &= 0xFFFFFFFF; // FIXME: still have port names here > - kmsg->msgh_local_port &= 0xFFFFFFFF; // also, this assumes little-endian > -#endif > > vm_offset_t usaddr, ueaddr, ksaddr; > 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)) > { > @@ -441,7 +391,6 @@ int copyinmsg (const void *userbuf, void *kernelbuf, > const size_t usize, const s > { > if (MACH_MSG_TYPE_PORT_ANY(name)) > { > -#ifdef USER32 > if (size != bytes_to_descsize(sizeof(mach_port_name_t))) > return 1; > if ((usaddr + sizeof(mach_port_name_t)*number) > ueaddr) > @@ -454,17 +403,6 @@ int copyinmsg (const void *userbuf, void *kernelbuf, > const size_t usize, const s > ksaddr += sizeof(mach_port_t); > usaddr += sizeof(mach_port_name_t); > } > -#else > - if (size != > bytes_to_descsize(sizeof(mach_port_name_inlined_t))) > - return 1; > - const vm_size_t length = number * > sizeof(mach_port_name_inlined_t); > - if ((usaddr + length) > ueaddr) > - return 1; > - if (copyin((void*)usaddr, (void*)ksaddr, length)) > - return 1; > - usaddr += length; > - ksaddr += length; > -#endif > } > else > { > @@ -485,11 +423,9 @@ int copyinmsg (const void *userbuf, void *kernelbuf, > const size_t usize, const s > > /* out-of-line port arrays are always arrays of > mach_port_name_t (4 bytes) > * and are expanded in ipc_kmsg_copyin_body() */ > - if (MACH_MSG_TYPE_PORT_ANY(name)) { > - if (size != bytes_to_descsize(sizeof(mach_port_name_t))) > - return 1; > - adjust_msg_type_size(ktaddr, sizeof(mach_port_t) - > sizeof(mach_port_name_t)); > - } > + if (MACH_MSG_TYPE_PORT_ANY(name) && > + (size != bytes_to_descsize(sizeof(mach_port_name_t)))) > + return 1; > > if (copyin_address((rpc_vm_offset_t*)usaddr, > (vm_offset_t*)ksaddr)) > return 1; > @@ -506,18 +442,24 @@ int copyinmsg (const void *userbuf, void *kernelbuf, > const size_t usize, const s > > kmsg->msgh_size = sizeof(mach_msg_header_t) + ksaddr - (vm_offset_t)(kmsg > + 1); > assert(kmsg->msgh_size <= ksize); > -#ifndef USER32 > - if (kmsg->msgh_size != usize) > +#else > + /* The 64 bit interface ensures the header is the same size, so it does > not need any resizing. */ > + _Static_assert(sizeof(mach_msg_header_t) == sizeof(mach_msg_user_header_t), > + "mach_msg_header_t and mach_msg_user_header_t expected to be > of the same size"); > + if (copyin(umsg, kmsg, usize)) > return 1; > + kmsg->msgh_remote_port &= 0xFFFFFFFF; // FIXME: still have port names here > + kmsg->msgh_local_port &= 0xFFFFFFFF; // also, this assumes little-endian > #endif > return 0; > } > > +#ifdef USER32 > +/* This is defined simply as copyout for !USER32. */ > int copyoutmsg (const void *kernelbuf, void *userbuf, const size_t ksize) > { > const mach_msg_header_t *kmsg = kernelbuf; > mach_msg_user_header_t *umsg = userbuf; > -#ifdef USER32 > if (copyout(&kmsg->msgh_bits, &umsg->msgh_bits, sizeof(kmsg->msgh_bits))) > return 1; > /* umsg->msgh_size is filled in later */ > @@ -528,10 +470,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, > const size_t ksize) > if (copyout(&kmsg->msgh_seqno, &umsg->msgh_seqno, > sizeof(kmsg->msgh_seqno) + sizeof(kmsg->msgh_id))) > return 1; > -#else > - if (copyout(kmsg, umsg, sizeof(mach_msg_header_t))) > - return 1; > -#endif /* USER32 */ > > vm_offset_t ksaddr, keaddr, usaddr; > ksaddr = (vm_offset_t)(kmsg + 1); > @@ -559,7 +497,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, > const size_t ksize) > { > if (MACH_MSG_TYPE_PORT_ANY(name)) > { > -#ifdef USER32 > for (int i=0; i<number; i++) > { > if (copyout_port((mach_port_t*)ksaddr, > (mach_port_name_t*)usaddr)) > @@ -567,15 +504,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, > const size_t ksize) > ksaddr += sizeof(mach_port_t); > usaddr += sizeof(mach_port_name_t); > } > -#else > - if (size != > bytes_to_descsize(sizeof(mach_port_name_inlined_t))) > - return 1; > - const vm_size_t length = number * > sizeof(mach_port_name_inlined_t); > - if (copyout((void*)ksaddr, (void*)usaddr, length)) > - return 1; > - ksaddr += length; > - usaddr += length; > -#endif > } > else > { > @@ -603,11 +531,6 @@ int copyoutmsg (const void *kernelbuf, void *userbuf, > const size_t ksize) > usize = sizeof(mach_msg_user_header_t) + usaddr - (vm_offset_t)(umsg + 1); > if (copyout(&usize, &umsg->msgh_size, sizeof(umsg->msgh_size))) > return 1; > -#ifndef USER32 > - if (usize != ksize) > - return 1; > -#endif > - > return 0; > - > } > +#endif /* USER32 */ > -- > 2.39.2 > >
-- Samuel --- Pour une évaluation indépendante, transparente et rigoureuse ! Je soutiens la Commission d'Évaluation de l'Inria.