From d5019a13981126eaee7677bdf5cfc68d48e5a909 Mon Sep 17 00:00:00 2001 From: Irek Fakhrutdinov Date: Thu, 12 Dec 2024 12:32:33 +0100 Subject: [PATCH] Allocate executable storage properly Some storage that is intended to be executed is not allocated with the EXECUTABLE=YES option, can lead to ABENDs when certain z/OS options are set. This commit changes the code which allocates such storage to allocate it with EXECUTABLE=YES. Signed-off-by: Irek Fakhrutdinov --- CHANGELOG.md | 1 + c/cmutils.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++++ c/crossmemory.c | 25 +++++++++++------- c/qsam.c | 23 +++++++++++++++-- h/cmutils.h | 63 +++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 169 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d7ede9fa..7b2e7f4f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Enhancement: Adding more arguments to httpClientSessionInit to allow passing back internal rc and removing the reference from changelog in `3.0.0`. (#499). - Bugfix: make sure CEE3ERP is invoked in LE 31-bit XPLINK (#504) +- Bugfix: allocate storage to be executed with EXECUTABLE=YES (#xxx) ## `3.0.0` - Feature: added javascript `zos.getStatvfs(path)` function to obtain file system information (#482). diff --git a/c/cmutils.c b/c/cmutils.c index cd02a62ca..76b25bfa1 100644 --- a/c/cmutils.c +++ b/c/cmutils.c @@ -324,6 +324,16 @@ void cmFree(void *data, unsigned int size, int subpool, int key) { cmFree2(&data, size, subpool, key); } +void *cmAllocExec(unsigned int size, int subpool, int key) { + void *data = NULL; + cmAlloc2Exec(size, subpool, key, &data); + return data; +} + +void cmFreeExec(void *data, unsigned int size, int subpool, int key) { + cmFree2Exec(&data, size, subpool, key); +} + void cmAlloc2(unsigned int size, int subpool, int key, void **resultPtr) { *resultPtr = NULL; @@ -381,6 +391,64 @@ void cmFree2(void **dataPtr, unsigned int size, int subpool, int key) { } +void cmAlloc2Exec(unsigned int size, int subpool, int key, void **resultPtr) { + + *resultPtr = NULL; + key <<= 4; + + // Note: Used ADDR= option so STORAGE macro stores the address (for use by + // recovery) a few instructions sooner than the compiler would. +#ifndef _LP64 +#define ROFF "0" +#else +#define ROFF "4" +#endif + + __asm( + ASM_PREFIX + " STORAGE OBTAIN" + ",COND=YES" + ",LENGTH=(%[size])" + ",ADDR="ROFF"(,%[resultPtr])" + ",SP=(%[sp])" + ",LOC=31" + ",OWNER=SYSTEM" + ",KEY=(%[key])" + ",EXECUTABLE=YES" + ",LINKAGE=SYSTEM \n" + : [result]"=m"(*resultPtr) + : [size]"NR:r0"(size), [sp]"r"(subpool), [key]"r"(key), + [resultPtr]"r"(resultPtr) + : "r1", "r14", "r15" + ); + + +} + +void cmFree2Exec(void **dataPtr, unsigned int size, int subpool, int key) { + + void *localData = *dataPtr; + *dataPtr = NULL; + key <<= 4; + + __asm( + ASM_PREFIX + " STORAGE RELEASE" + ",COND=NO" + ",LENGTH=(%[size])" + ",SP=(%[sp])" + ",ADDR=(%[storageAddr])" + ",KEY=(%[key])" + ",EXECUTABLE=YES" + ",LINKAGE=SYSTEM \n" + : + : [size]"NR:r0"(size), [storageAddr]"r"(localData), + [sp]"r"(subpool), [key]"r"(key) + : "r1", "r14", "r15" + ); + +} + ZOWE_PRAGMA_PACK typedef int32_t CPID; diff --git a/c/crossmemory.c b/c/crossmemory.c index 6c529658a..be3b47c2c 100644 --- a/c/crossmemory.c +++ b/c/crossmemory.c @@ -2168,6 +2168,14 @@ static void freeECSAStorage(void *data, unsigned int size) { cmFree(data, size, CROSS_MEMORY_SERVER_SUBPOOL, CROSS_MEMORY_SERVER_KEY); } +static void *allocateECSAStorageExec(unsigned int size) { + return cmAllocExec(size, CROSS_MEMORY_SERVER_SUBPOOL, CROSS_MEMORY_SERVER_KEY); +} + +static void freeECSAStorageExec(void *data, unsigned int size) { + cmFreeExec(data, size, CROSS_MEMORY_SERVER_SUBPOOL, CROSS_MEMORY_SERVER_KEY); +} + static void allocateECSAStorage2(unsigned int size, void **resultPtr) { @@ -3112,11 +3120,10 @@ static CMSLookupRoutineAnchor *makeLookupRoutineAnchor(void) { anchorLength, sizeof(CMSLookupRoutineAnchor)); return NULL; } - // TODO if the cmutils alloc functions are changed to allocate non-executable - // storage, this call will need to change - CMSLookupRoutineAnchor *anchor = cmAlloc(sizeof(CMSLookupRoutineAnchor), - CMS_LOOKUP_ANCHOR_SUBPOOL, - CMS_LOOKUP_ANCHOR_KEY); + + CMSLookupRoutineAnchor *anchor = cmAllocExec(sizeof(CMSLookupRoutineAnchor), + CMS_LOOKUP_ANCHOR_SUBPOOL, + CMS_LOOKUP_ANCHOR_KEY); if (anchor == NULL) { zowelog(NULL, LOG_COMP_ID_CMS, ZOWE_LOG_DEBUG, CMS_LOG_DEBUG_MSG_ID" Alloc failed for look-up anchor\n"); @@ -3156,8 +3163,8 @@ static CMSLookupRoutineAnchor *makeLookupRoutineAnchor(void) { } static void deleteLookupRoutineAnchor(CMSLookupRoutineAnchor *anchor) { - cmFree(anchor, anchor->size, CMS_LOOKUP_ANCHOR_SUBPOOL, - CMS_LOOKUP_ANCHOR_KEY); + cmFreeExec(anchor, anchor->size, CMS_LOOKUP_ANCHOR_SUBPOOL, + CMS_LOOKUP_ANCHOR_KEY); } static int discardLookupRoutineAnchor(CrossMemoryServer *server) { @@ -4671,7 +4678,7 @@ static int installResourceManager(CrossMemoryServerGlobalArea *globalArea, Resou resmgrRoutine, resmgrRoutineSize); zowedump(NULL, LOG_COMP_ID_CMS, ZOWE_LOG_DEBUG, resmgrRoutine, resmgrRoutineSize); - resmgrRoutineInECSA = allocateECSAStorage(resmgrRoutineSize); + resmgrRoutineInECSA = allocateECSAStorageExec(resmgrRoutineSize); if (resmgrRoutineInECSA == NULL) { zowelog(NULL, LOG_COMP_ID_CMS, ZOWE_LOG_SEVERE, CMS_LOG_RESMGR_ECSA_FAILURE_MSG, resmgrRoutineSize); status = RC_CMS_ECSA_ALLOC_FAILED; @@ -4687,7 +4694,7 @@ static int installResourceManager(CrossMemoryServerGlobalArea *globalArea, Resou int createTokenRC = nameTokenCreatePersistent(NAME_TOKEN_LEVEL_SYSTEM, &resmgrRoutineNTName, &resmgrRoutineNTToken.token); if (createTokenRC != 0) { zowelog(NULL, LOG_COMP_ID_CMS, ZOWE_LOG_SEVERE, CMS_LOG_RESMGR_NT_NOT_CREATED_MSG, createTokenRC); - freeECSAStorage(resmgrRoutineInECSA, resmgrRoutineSize); + freeECSAStorageExec(resmgrRoutineInECSA, resmgrRoutineSize); resmgrRoutineInECSA = NULL; status = RC_CMS_NAME_TOKEN_CREATE_FAILED; } diff --git a/c/qsam.c b/c/qsam.c index 7fdac4c08..ad2de1529 100644 --- a/c/qsam.c +++ b/c/qsam.c @@ -40,6 +40,16 @@ char *malloc24(int size){ return data; } +static void *malloc24Exec(int size) { + char *data; + __asm(ASM_PREFIX + " STORAGE OBTAIN,LENGTH=(%[length]),LOC=24,EXECUTABLE=YES\n" + : [address]"=NR:r1"(data) + : [length]"NR:r0"(size) + : "r14", "r15"); + return data; +} + static char *malloc31(int size){ char *data; __asm(ASM_PREFIX @@ -61,6 +71,15 @@ char *free24(char *data, int size){ return NULL; } +static void free24Exec(void *data, int size) { + __asm(ASM_PREFIX + " STORAGE RELEASE,LENGTH=(%[length]),ADDR=(%[address]),CALLRKY=YES" + ",EXECUTABLE=YES\n" + : + : [length]"NR:r0"(size), [address]"NR:r1"(data) + : "r14", "r15"); +} + static void free31(void *data, int size){ /* printf("attempting to free, size: %d\n", size);*/ __asm(ASM_PREFIX @@ -446,7 +465,7 @@ static int openCloseBBQ(char *dcb, int mode, int svc){ reqDcb->dcb.Common.macroFormat != DCB_MACRF_EXCP && reqDcb->dcb.BBQCommon.blksize_s <= 0) { - openexitData = (openexitData_t *)malloc24(sizeof(openexitData_t)); + openexitData = malloc24Exec(sizeof(openexitData_t)); memset(openexitData,0,sizeof(openexitData_t)); /* If a block size of -2 is specified as the value for the block @@ -766,7 +785,7 @@ static int openCloseBBQ(char *dcb, int mode, int svc){ if (openexitData){ reqDcb->dcb.Common.exlst = (unsigned int)NULL; - free24((char *)openexitData,sizeof(openexitData_t)); + free24Exec(openexitData,sizeof(openexitData_t)); } } else{ __asm(ASM_PREFIX diff --git a/h/cmutils.h b/h/cmutils.h index ff7bd01e9..50a594e79 100644 --- a/h/cmutils.h +++ b/h/cmutils.h @@ -47,6 +47,10 @@ #define cmFree CMFREE #define cmAlloc2 CMALLOC2 #define cmFree2 CMFREE2 +#define cmAllocExec CMALLCX +#define cmFreeExec CMFREEX +#define cmAlloc2Exec CMALLC2X +#define cmFree2Exec CMFREE2X #define makeCrossMemoryMap CMUMKMAP #define removeCrossMemoryMap CMURMMAP @@ -208,6 +212,30 @@ void *cmAlloc(unsigned int size, int subpool, int key); */ void cmFree(void *data, unsigned int size, int subpool, int key); +/** + * @brief Allocates executable storage. The function can be used in cross-memory + * mode. + * + * @param size Size of the storage. + * @param subpool Subpool of the storage. + * @param key Key of the storage + * @return The address of the storage in case of success, NULL if the request + * has failed. + */ +void *cmAllocExec(unsigned int size, int subpool, int key); + +/** + * @brief Releases executable storage. The call is unconditional, that is, it + * will ABEND + * if bad storage is passed. The function can be used in cross-memory mode. + * + * @param data Storage to be released. + * @param size Size of the storage. + * @param subpool Subpool of the storage. + * @param key Key of the storage. + */ +void cmFreeExec(void *data, unsigned int size, int subpool, int key); + /** * @brief Allocates storage. The function can be used in cross-memory mode. * @@ -241,6 +269,41 @@ void cmAlloc2(unsigned int size, int subpool, int key, void **resultPtr); */ void cmFree2(void **dataPtr, unsigned int size, int subpool, int key); +/** + * @brief Allocates executable storage. The function can be used in cross-memory + * mode. + * + * This version should be used in code which uses recovery as the window between + * the actual allocation and returning the result is much smaller than in + * cmAlloc. + * + * @param size Size of the storage. + * @param subpool Subpool of the storage. + * @param key Key of the storage. + * @param resultPtr Pointer to the result address of the storage. The pointer is + * set to NULL if the request fails. + */ +void cmAlloc2Exec(unsigned int size, int subpool, int key, void **resultPtr); + +/** + * @brief Releases executable storage. The function can be used in cross-memory + * mode. + * + * The call is unconditional, that is, it will ABEND if bad storage is passed. + * This version should be used in code which uses recovery as it first sets + * the provided address to NULL and then releases the storage, whereby + * helping to prevent double-free. If an ABEND happens in between setting + * provided address to NULL and releasing the storage, the storage will be + * leaked. + * + * @param dataPtr Pointer to the address of the storage to be released. It will + * be set to NULL. + * @param size Size of the storage. + * @param subpool Subpool of the storage. + * @param key Key of the storage. + */ +void cmFree2Exec(void **dataPtr, unsigned int size, int subpool, int key); + typedef struct CrossMemoryMap_tag CrossMemoryMap; /**