From f9835a93bd7eda372cb9f769f1dc3e6f98d266aa Mon Sep 17 00:00:00 2001 From: Yosuke Shimizu Date: Wed, 22 Jul 2026 17:10:28 +0900 Subject: [PATCH] Bound POSIX SHM mapping by the shared object size --- port/posix/posix_transport_shm.c | 120 ++++++++++++----- port/posix/posix_transport_shm.h | 32 +++++ test/wh_test_comm.c | 216 ++++++++++++++++++++++++++++++- 3 files changed, 336 insertions(+), 32 deletions(-) diff --git a/port/posix/posix_transport_shm.c b/port/posix/posix_transport_shm.c index a61863dcd..a52bfbec9 100644 --- a/port/posix/posix_transport_shm.c +++ b/port/posix/posix_transport_shm.c @@ -73,7 +73,7 @@ enum { /** Local declarations */ /* Memory map and interpret the header block */ -static int posixTransportShm_Map(int fd, size_t size, ptshmMapping* map); +static int posixTransportShm_Map(int fd, ptshmMapping* map); #if defined(WOLFHSM_CFG_ENABLE_SERVER) /* Create and map a shared object for transport */ @@ -91,30 +91,84 @@ static int posixTransportShm_HandleMap(posixTransportShmContext* ctx); #endif /** Local Definitions */ -static int posixTransportShm_Map(int fd, size_t size, ptshmMapping* map) +static int posixTransportShm_Map(int fd, ptshmMapping* map) { - int ret = WH_ERROR_OK; - void* ptr = NULL; + int ret = WH_ERROR_OK; + void* ptr = NULL; + ptshmHeader* header = NULL; + struct stat st[1] = {0}; + size_t size = 0; + size_t total = 0; + size_t map_len = 0; + size_t dma_size = 0; + uint16_t req_size = 0; + uint16_t resp_size = 0; if ((fd < 0) || (map == NULL)) { return WH_ERROR_BADARGS; } - /* Map the shared memory object */ - ptr = mmap(NULL, size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); - if (ptr != MAP_FAILED) { - memset(map, 0, sizeof(*map)); - map->ptr = ptr; - map->size = size; - map->header = (ptshmHeader*)ptr; - map->req = (uint8_t*)(map->header + 1); - map->resp = map->req + map->header->req_size; - map->dma = map->resp + map->header->resp_size; - map->dma_size = map->header->dma_size; - } - else { + /* The object size bounds the layout; the header is never trusted for it */ + if (fstat(fd, st) != 0) { ret = WH_ERROR_ABORTED; } + else if (st->st_size < (off_t)sizeof(ptshmHeader)) { + ret = WH_ERROR_BADARGS; + } + + /* Snapshot the peer-writable sizes from a header-only mapping, so the full + * mapping can be sized from them rather than the object size. */ + if (ret == WH_ERROR_OK) { + size = (size_t)st->st_size; + header = (ptshmHeader*)mmap(NULL, sizeof(ptshmHeader), PROT_READ, + MAP_SHARED, fd, 0); + if (header == MAP_FAILED) { + ret = WH_ERROR_ABORTED; + } + } + + if (ret == WH_ERROR_OK) { + req_size = header->req_size; + resp_size = header->resp_size; + dma_size = header->dma_size; + (void)munmap((void*)header, sizeof(ptshmHeader)); + header = NULL; + + /* Each buffer must be a whole number of control/status words, or the + * transport underflows a length check or misaligns a pointer. total + * cannot overflow, so dma_size checks against the remainder. */ + total = sizeof(ptshmHeader) + (size_t)req_size + (size_t)resp_size; + if ((req_size < sizeof(whTransportMemCsr)) || + ((req_size % sizeof(whTransportMemCsr)) != 0) || + (resp_size < sizeof(whTransportMemCsr)) || + ((resp_size % sizeof(whTransportMemCsr)) != 0) || (total > size) || + (dma_size > (size - total))) { + ret = WH_ERROR_BADARGS; + } + } + + if (ret == WH_ERROR_OK) { + /* Only the declared footprint, so padding past it is never mapped */ + map_len = total + dma_size; + ptr = mmap(NULL, map_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (ptr == MAP_FAILED) { + ret = WH_ERROR_ABORTED; + } + } + + if (ret == WH_ERROR_OK) { + header = (ptshmHeader*)ptr; + memset(map, 0, sizeof(*map)); + map->ptr = ptr; + map->size = map_len; + map->header = header; + map->req = (uint8_t*)(map->header + 1); + map->resp = map->req + req_size; + map->dma = map->resp + resp_size; + map->req_size = req_size; + map->resp_size = resp_size; + map->dma_size = dma_size; + } return ret; } @@ -139,13 +193,7 @@ static int posixTransportShm_UseMap(char* name, ptshmMapping* map) ptshmHeader* header = (ptshmHeader*)mmap( NULL, sizeof(*header), PROT_READ, MAP_SHARED, fd, 0); if (header != MAP_FAILED) { - size_t size = 0; - if (header->initialized == PTSHM_INITIALIZED_CREATOR) { - /* Read provided sizes */ - size = sizeof(*header) + header->req_size + - header->resp_size + header->dma_size; - } - else { + if (header->initialized != PTSHM_INITIALIZED_CREATOR) { /* Header not configured */ ret = WH_ERROR_NOTREADY; } @@ -153,7 +201,7 @@ static int posixTransportShm_UseMap(char* name, ptshmMapping* map) (void)munmap((void*)header, sizeof(*header)); if (ret == WH_ERROR_OK) { - ret = posixTransportShm_Map(fd, size, map); + ret = posixTransportShm_Map(fd, map); if (ret == WH_ERROR_OK) { /* Unlnk the object */ (void)shm_unlink(name); @@ -202,7 +250,13 @@ static int posixTransportShm_CreateMap(char* name, uint16_t req_size, int ret = WH_ERROR_OK; int fd = -1; - if ((name == NULL) || (map == NULL)) { + /* Apply the same buffer size rule to the configuration before the object + * exists, so a misconfigured server cannot leave a stale object behind */ + if ((name == NULL) || (map == NULL) || + (req_size < sizeof(whTransportMemCsr)) || + ((req_size % sizeof(whTransportMemCsr)) != 0) || + (resp_size < sizeof(whTransportMemCsr)) || + ((resp_size % sizeof(whTransportMemCsr)) != 0)) { return WH_ERROR_BADARGS; } @@ -229,7 +283,11 @@ static int posixTransportShm_CreateMap(char* name, uint16_t req_size, /* Unmap the header and remap the full area */ (void)munmap((void*)header, sizeof(*header)); - ret = posixTransportShm_Map(fd, size, map); + ret = posixTransportShm_Map(fd, map); + if (ret != WH_ERROR_OK) { + /* Don't leave an object a client would keep rejecting */ + (void)shm_unlink(name); + } } else { /* Problem mapping the header */ @@ -268,8 +326,8 @@ static int posixTransportShm_HandleMap(posixTransportShmContext* ctx) ret = posixTransportShm_UseMap(ctx->name, map); if (ret == WH_ERROR_OK) { /* Configure the underlying transport context */ - tMemCfg->req_size = map->header->req_size; - tMemCfg->resp_size = map->header->resp_size; + tMemCfg->req_size = map->req_size; + tMemCfg->resp_size = map->resp_size; tMemCfg->req = map->req; tMemCfg->resp = map->resp; @@ -405,8 +463,8 @@ int posixTransportShm_ServerInit(void* c, const void* cf, ctx->connectcb_arg = connectcb_arg; /* Configure the underlying transport context */ - tMemCfg->req_size = map->header->req_size; - tMemCfg->resp_size = map->header->resp_size; + tMemCfg->req_size = map->req_size; + tMemCfg->resp_size = map->resp_size; tMemCfg->req = map->req; tMemCfg->resp = map->resp; diff --git a/port/posix/posix_transport_shm.h b/port/posix/posix_transport_shm.h index edbc50811..f2767740d 100644 --- a/port/posix/posix_transport_shm.h +++ b/port/posix/posix_transport_shm.h @@ -48,6 +48,38 @@ * versions of requests by configuring the base address of the DMA request to be * the mapped address of the DMA block. * + * req_size and resp_size must each be a non-zero multiple of + * sizeof(whTransportMemCsr), since every buffer begins with a control/status + * word and the buffer that follows must stay aligned. Server init fails with + * WH_ERROR_BADARGS otherwise, and a client rejects an object whose header + * declares sizes that do not meet the same rule. + * + * Threat model + * + * This transport is intended for testing, debugging, and as a reference port. + * It is not a security boundary and must not be used where the client and the + * server belong to different trust domains. Specifically: + * + * - Same trust domain assumed. Both peers are expected to run under the same + * uid, and each is trusted with the contents and the lifetime of the shared + * object. Anything a peer places in the shared memory is trusted input. + * + * - No peer authentication. Whichever process opens the named object is taken + * to be the peer. A process that wins the race to create the name can + * interpose between a client and a server, and the name is unlinked by the + * client on successful mapping, so it can be recreated by anyone afterwards. + * + * - No isolation and no availability guarantee. A peer may ftruncate() the + * object out from under the other side, which faults on the next access, or + * simply never advance the control/status words and stall the exchange. + * + * The header fields are validated against the real size of the shared object, + * so a malformed or truncated object cannot steer this process into an + * out-of-bounds access. That is robustness against a corrupt object, not + * protection against a hostile peer, and it does not make this transport a + * trusted channel. Use a transport with an actual isolation boundary when the + * peer is not trusted. + * */ #ifndef PORT_POSIX_POSIX_TRANSPORT_SHM_H_ diff --git a/test/wh_test_comm.c b/test/wh_test_comm.c index fb6132aa0..0b77304d5 100644 --- a/test/wh_test_comm.c +++ b/test/wh_test_comm.c @@ -44,7 +44,10 @@ #if defined(WOLFHSM_CFG_TEST_POSIX) #include /* For pthread_create/cancel/join/_t */ #include -#include /* For nanosleep */ +#include /* For nanosleep */ +#include /* For O_* constants */ +#include /* For shm_open, mmap */ +#include /* For mode constants */ #include "port/posix/posix_transport_tcp.h" #include "port/posix/posix_transport_shm.h" @@ -539,6 +542,211 @@ void wh_CommClientServer_ShMemThreadTest(void) _whCommClientServerThreadTest(c_conf, s_conf); } +/* Mirror of the file-local ptshmHeader in port/posix/posix_transport_shm.c, so + * the test can write layouts a well-behaved creator would never produce. Drift + * shows up as the accept case below failing to connect. */ +#define PTSHM_TEST_HEADER_SIZE 64 +#define PTSHM_TEST_INITIALIZED_CREATOR 2 +/* Larger than any object the test creates, and a valid buffer size, so the + * layout is refused on the total-versus-object bound and nothing else */ +#define PTSHM_TEST_OVERSIZE_BUFFER 65528 + +/* How the squat helper derives the dma_size it writes */ +enum { + PTSHM_TEST_DMA_GIVEN = 0, /* use the caller's dma_size verbatim */ + PTSHM_TEST_DMA_EXACT, /* exactly fill the object: largest accepted */ + PTSHM_TEST_DMA_OVER /* one byte more than the object can hold */ +}; + +typedef union { + struct { + uint32_t initialized; + uint16_t req_size; + uint16_t resp_size; + size_t dma_size; + pid_t creator_pid; + pid_t user_pid; + } f; + uint8_t pad[PTSHM_TEST_HEADER_SIZE]; +} ptshmTestHeader; + +/* Squat the name with a caller-supplied header, then let the client open it. + * Returns the client Init result, or WH_ERROR_ABORTED on setup failure. */ +static int _whTest_CommShmSquat(char* name, off_t obj_size, uint16_t req_size, + uint16_t resp_size, size_t dma_size, + int dma_mode) +{ + int ret = WH_ERROR_OK; + int fd = -1; + ptshmTestHeader* hdr = NULL; + struct stat st[1] = {0}; + whCommConnected connected = WH_COMM_DISCONNECTED; + whTransportClientCb cb[1] = {POSIX_TRANSPORT_SHM_CLIENT_CB}; + posixTransportShmClientContext ctx[1] = {0}; + posixTransportShmConfig cfg[1] = {0}; + + cfg->name = name; + + (void)shm_unlink(name); + fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, 0600); + if (fd < 0) { + WH_ERROR_PRINT("shm_open(%s) failed\n", name); + return WH_ERROR_ABORTED; + } + + if (ftruncate(fd, obj_size) != 0) { + WH_ERROR_PRINT("ftruncate(%s) failed\n", name); + ret = WH_ERROR_ABORTED; + } + + /* The OS may round the object up, so the sizes that sit on the accept and + * reject sides of the bound come from the size it actually ended up with */ + if ((ret == WH_ERROR_OK) && (dma_mode != PTSHM_TEST_DMA_GIVEN)) { + if (fstat(fd, st) != 0) { + WH_ERROR_PRINT("fstat(%s) failed\n", name); + ret = WH_ERROR_ABORTED; + } + else { + dma_size = (size_t)st->st_size - sizeof(ptshmTestHeader) - + (size_t)req_size - (size_t)resp_size; + if (dma_mode == PTSHM_TEST_DMA_OVER) { + dma_size++; + } + } + } + + if (ret == WH_ERROR_OK) { + hdr = (ptshmTestHeader*)mmap(NULL, sizeof(*hdr), + PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); + if (hdr == MAP_FAILED) { + WH_ERROR_PRINT("mmap(%s) failed\n", name); + ret = WH_ERROR_ABORTED; + } + } + + if (ret == WH_ERROR_OK) { + hdr->f.req_size = req_size; + hdr->f.resp_size = resp_size; + hdr->f.dma_size = dma_size; + hdr->f.creator_pid = getpid(); + hdr->f.initialized = PTSHM_TEST_INITIALIZED_CREATOR; + (void)munmap((void*)hdr, sizeof(*hdr)); + + ret = cb->Init(ctx, cfg, NULL, NULL); + + /* Init folds NOTFOUND and NOTREADY into OK, so an OK return alone does + * not mean the object was mapped. Report never-mapped as NOTREADY. */ + if (ret == WH_ERROR_OK) { + if ((posixTransportShm_IsConnected(ctx, &connected) != + WH_ERROR_OK) || + (connected != WH_COMM_CONNECTED)) { + ret = WH_ERROR_NOTREADY; + } + } + (void)cb->Cleanup(ctx); + } + + (void)close(fd); + (void)shm_unlink(name); + return ret; +} + +/* A creator that races to claim the name can describe a layout that does not + * fit the object it created. Every such layout must be refused. */ +static int _whTest_CommShmMalformedHeader(void) +{ + int ret = WH_ERROR_OK; + char name[48] = {0}; + off_t obj_size = 0; + + obj_size = sizeof(ptshmTestHeader) + REQ_SIZE + RESP_SIZE + BUFFER_SIZE; + + (void)snprintf(name, sizeof(name), "/wh_test_comm_shm_bad.%u", + (unsigned)getpid()); + + /* Exactly fills the object: the largest dma_size that must be accepted */ + ret = _whTest_CommShmSquat(name, obj_size, REQ_SIZE, RESP_SIZE, 0, + PTSHM_TEST_DMA_EXACT); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_OK); + + /* Object far larger than the declared footprint is still accepted; the + * transport maps only the footprint and ignores the padding */ + ret = _whTest_CommShmSquat(name, obj_size, REQ_SIZE, RESP_SIZE, 0, + PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_OK); + + /* One byte more than the object holds: the smallest that must be refused */ + ret = _whTest_CommShmSquat(name, obj_size, REQ_SIZE, RESP_SIZE, 0, + PTSHM_TEST_DMA_OVER); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + /* dma_size larger than the bytes left after the header and buffers */ + ret = _whTest_CommShmSquat(name, obj_size, REQ_SIZE, RESP_SIZE, ~(size_t)0, + PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + /* Valid buffer sizes whose total still overruns the object */ + ret = _whTest_CommShmSquat(name, obj_size, PTSHM_TEST_OVERSIZE_BUFFER, + PTSHM_TEST_OVERSIZE_BUFFER, 0, + PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + /* Buffers too small for their control word */ + ret = _whTest_CommShmSquat(name, obj_size, sizeof(whTransportMemCsr) - 1, + RESP_SIZE, 0, PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + ret = _whTest_CommShmSquat(name, obj_size, REQ_SIZE, + sizeof(whTransportMemCsr) - 1, 0, + PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + /* Buffers that are not a whole number of control words */ + ret = _whTest_CommShmSquat(name, obj_size, sizeof(whTransportMemCsr) + 1, + RESP_SIZE, 0, PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + ret = _whTest_CommShmSquat(name, obj_size, REQ_SIZE, + sizeof(whTransportMemCsr) + 1, 0, + PTSHM_TEST_DMA_GIVEN); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + return WH_TEST_SUCCESS; +} + +/* The same rule applies to the server's own configuration, and a rejected + * config must not leave an object behind for clients to trip over. */ +static int _whTest_CommShmServerBadConfig(void) +{ + int ret = WH_ERROR_OK; + int fd = -1; + char name[48] = {0}; + whTransportServerCb cb[1] = {POSIX_TRANSPORT_SHM_SERVER_CB}; + posixTransportShmServerContext ctx[1] = {0}; + posixTransportShmConfig cfg[1] = {0}; + + (void)snprintf(name, sizeof(name), "/wh_test_comm_shm_cfg.%u", + (unsigned)getpid()); + cfg->name = name; + cfg->req_size = sizeof(whTransportMemCsr) + 1; + cfg->resp_size = RESP_SIZE; + cfg->dma_size = 0; + + ret = cb->Init(ctx, cfg, NULL, NULL); + WH_TEST_ASSERT_RETURN(ret == WH_ERROR_BADARGS); + + /* Nothing should be left linked under that name */ + fd = shm_open(name, O_RDONLY, 0); + if (fd >= 0) { + (void)close(fd); + (void)shm_unlink(name); + WH_ERROR_PRINT("server left %s linked after a rejected config\n", name); + return WH_TEST_FAIL; + } + + return WH_TEST_SUCCESS; +} + void wh_CommClientServer_TcpThreadTest(void) { posixTransportTcpConfig mytcpconfig[1] = {{ @@ -588,6 +796,12 @@ int whTest_Comm(void) WH_TEST_PRINT("Testing comms: (pthread) posix mem...\n"); wh_CommClientServer_ShMemThreadTest(); + + WH_TEST_PRINT("Testing comms: posix shm malformed header...\n"); + WH_TEST_RETURN_ON_FAIL(_whTest_CommShmMalformedHeader()); + + WH_TEST_PRINT("Testing comms: posix shm bad server config...\n"); + WH_TEST_RETURN_ON_FAIL(_whTest_CommShmServerBadConfig()); #endif /* defined(WOLFHSM_CFG_TEST_POSIX) */ return 0;