From d5821910164d69b4b831844d42b692dbe9fe672b Mon Sep 17 00:00:00 2001 From: Howard Pritchard Date: Mon, 28 Jul 2025 16:25:11 -0600 Subject: [PATCH] MPI_T: add code to check obj_handle arg Three MPI_T methods - for pvar, cvar, events, can bind one of these objects to an arbitrary MPI object. There was verbiage added to the MPI 4.1 standard which states its erroneous to bind one of these 'T' objects to an invalid MPI object. This PR adds code to make an effort at checking the validity of the arbitrary MPI object argument. We don't bother with events since only a stub implementation is currently present. Related to issue #12185. Signed-off-by: Howard Pritchard --- ompi/mpi/tool/cvar_handle_alloc.c | 8 ++ ompi/mpi/tool/cvar_read.c | 6 +- ompi/mpi/tool/mpit-internal.h | 7 ++ ompi/mpi/tool/mpit_common.c | 117 ++++++++++++++++++++++++++++++ ompi/mpi/tool/pvar_handle_alloc.c | 8 ++ 5 files changed, 144 insertions(+), 2 deletions(-) diff --git a/ompi/mpi/tool/cvar_handle_alloc.c b/ompi/mpi/tool/cvar_handle_alloc.c index 77514825a05..82ebe890032 100644 --- a/ompi/mpi/tool/cvar_handle_alloc.c +++ b/ompi/mpi/tool/cvar_handle_alloc.c @@ -9,6 +9,8 @@ * reserved. * Copyright (c) 2021 Amazon.com, Inc. or its affiliates. All Rights * reserved. + * Copyright (c) 2025 Triad National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -41,6 +43,12 @@ int MPI_T_cvar_handle_alloc (int cvar_index, void *obj_handle, return MPI_T_ERR_INVALID; } + if (NULL != obj_handle) { + if (ompit_obj_invalid(obj_handle)) { + return MPI_T_ERR_INVALID_HANDLE; + } + } + ompi_mpit_lock (); *handle = NULL; diff --git a/ompi/mpi/tool/cvar_read.c b/ompi/mpi/tool/cvar_read.c index b46db2c99c4..90b39d47a22 100644 --- a/ompi/mpi/tool/cvar_read.c +++ b/ompi/mpi/tool/cvar_read.c @@ -37,8 +37,10 @@ int MPI_T_cvar_read (MPI_T_cvar_handle handle, void *buf) return MPI_T_ERR_NOT_INITIALIZED; } - if (MPI_PARAM_CHECK && NULL == buf) { - return MPI_T_ERR_INVALID; + if (MPI_PARAM_CHECK) { + if ((NULL == buf) || (NULL == handle)) { + return MPI_T_ERR_INVALID; + } } ompi_mpit_lock (); diff --git a/ompi/mpi/tool/mpit-internal.h b/ompi/mpi/tool/mpit-internal.h index 4ee3d9e92a3..f8a6dda65d6 100644 --- a/ompi/mpi/tool/mpit-internal.h +++ b/ompi/mpi/tool/mpit-internal.h @@ -5,6 +5,8 @@ * Copyright (c) 2011 UT-Battelle, LLC. All rights reserved. * Copyright (c) 2017 IBM Corporation. All rights reserved. * Copyright (c) 2018 Cisco Systems, Inc. All rights reserved + * Copyright (c) 2025 Triad National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -22,8 +24,12 @@ #include "ompi/include/ompi_config.h" #include "ompi/runtime/params.h" #include "ompi/communicator/communicator.h" +#include "ompi/win/win.h" #include "ompi/constants.h" #include "ompi/datatype/ompi_datatype.h" +#include "ompi/file/file.h" +#include "ompi/op/op.h" +#include "ompi/message/message.h" #include "mpi.h" @@ -42,6 +48,7 @@ extern volatile uint32_t ompi_mpit_init_count; int ompit_var_type_to_datatype (mca_base_var_type_t type, MPI_Datatype *datatype); int ompit_opal_to_mpit_error (int rc); +bool ompit_obj_invalid(void *obj_handle); static inline int mpit_is_initialized (void) { diff --git a/ompi/mpi/tool/mpit_common.c b/ompi/mpi/tool/mpit_common.c index e47da814f7a..fc84440d14b 100644 --- a/ompi/mpi/tool/mpit_common.c +++ b/ompi/mpi/tool/mpit_common.c @@ -8,6 +8,8 @@ * Copyright (c) 2020 The University of Tennessee and The University * of Tennessee Research Foundation. All rights * reserved. + * Copyright (c) 2025 Triad National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -87,3 +89,118 @@ int ompit_opal_to_mpit_error (int rc) return MPI_T_ERR_INVALID; } } + +/* + * Check whether a MPI object is valid or not. + * If invalid return true, otherwise false. + */ +bool ompit_obj_invalid(void *obj_handle) +{ + bool ret = true; /* by default return obj is invalid */ + opal_object_t *opal_obj = NULL; + opal_class_t *opal_class; + const char *obj_name = NULL; + + if (NULL == obj_handle) { + goto fn_exit; + } + + /* we are actually evaulating a pointer to an OMPI MPI opaque handle */ + opal_obj = *(opal_object_t **)obj_handle; + + /* + * this should have already been checked by the caller but we do it here too + */ + if (NULL == opal_obj) { + goto fn_exit; + } + + opal_class = opal_obj->obj_class; + if (NULL == opal_class) { + goto fn_exit; + } + + obj_name = opal_class->cls_name; + if (NULL == obj_name) { + goto fn_exit; + } + + /* + * An MPIT C or P var or event can in principle be bound to any of these MPI object types + * This code is meant to address, at least in part, the text in section 15.3.2 in the MPI 4.1 + * standard concerning binding one of the T things to an MPI object. + */ + if (0 == strncmp(obj_name, "ompi_communicator_t", strlen("ompi_communicator_t"))) { + ompi_communicator_t *comm = (ompi_communicator_t *)obj_handle; + ret = ompi_comm_invalid(comm); + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_win_t", strlen("ompi_win_t"))) { + ompi_win_t *win = (ompi_win_t *)obj_handle; + ret = ompi_win_invalid(win); + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_file_t", strlen("ompi_file_t"))) { + ompi_file_t *file = (ompi_file_t *)obj_handle; + ret = ompi_file_invalid(file); + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_instance_t", strlen("ompi_instance_t"))) { + ompi_instance_t *instance = (ompi_instance_t *)obj_handle; + ret = ompi_instance_invalid(instance); + goto fn_exit; + } + + /* + * following object types don't seem to have robust validity checks so just + * do a smoke test for use of NULL objects. + */ + if (0 == strncmp(obj_name, "ompi_info_t", strlen("ompi_info_t"))) { + ompi_info_t *info = (ompi_info_t *)obj_handle; + ret = (MPI_INFO_NULL == info) ? true : false; + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_datatype_t", strlen("ompi_datatype_t"))) { + ompi_datatype_t *datatype = (ompi_datatype_t *)obj_handle; + ret = (MPI_DATATYPE_NULL == datatype) ? true : false; + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_request_t", strlen("ompi_request_t"))) { + ompi_request_t *request = (ompi_request_t *)obj_handle; + ret = (MPI_REQUEST_NULL == request) ? true : false; + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_errhandler_t", strlen("ompi_errhandler_t"))) { + ompi_errhandler_t *errhandler = (ompi_errhandler_t *)obj_handle; + ret = (MPI_ERRHANDLER_NULL == errhandler) ? true : false; + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_opt_t", strlen("ompi_op_t"))) { + ompi_op_t *op = (ompi_op_t *)obj_handle; + ret = (MPI_OP_NULL == op) ? true : false; + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_message_t", strlen("ompi_message_t"))) { + ompi_message_t *message = (ompi_message_t *)obj_handle; + ret = (MPI_MESSAGE_NULL == message) ? true : false; + goto fn_exit; + } + + if (0 == strncmp(obj_name, "ompi_group_t", strlen("ompi_group_t"))) { + ompi_group_t *group = (ompi_group_t *)obj_handle; + ret = (MPI_GROUP_NULL == group) ? true : false; + goto fn_exit; + } + +fn_exit: + return ret; +} + diff --git a/ompi/mpi/tool/pvar_handle_alloc.c b/ompi/mpi/tool/pvar_handle_alloc.c index 0400025a11c..b37e05a57e2 100644 --- a/ompi/mpi/tool/pvar_handle_alloc.c +++ b/ompi/mpi/tool/pvar_handle_alloc.c @@ -9,6 +9,8 @@ * reserved. * Copyright (c) 2021 Amazon.com, Inc. or its affiliates. All Rights * reserved. + * Copyright (c) 2025 Triad National Security, LLC. All rights + * reserved. * $COPYRIGHT$ * * Additional copyrights may follow @@ -37,6 +39,12 @@ int MPI_T_pvar_handle_alloc(MPI_T_pvar_session session, int pvar_index, return MPI_T_ERR_NOT_INITIALIZED; } + if (NULL != obj_handle) { + if (ompit_obj_invalid(obj_handle)) { + return MPI_T_ERR_INVALID_HANDLE; + } + } + ompi_mpit_lock (); do {