Skip to content

Fix client *Dma address translation (KeyExport / NvmAdd / NvmRead) and KeyCacheDma use-after-free #403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bigbrett merged 4 commits into from
Jun 11, 2026
Merged

Fix client *Dma address translation (KeyExport / NvmAdd / NvmRead) and KeyCacheDma use-after-free #403

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d1c2baa
client DMA: fix KeyCacheDma use-after-free and translate key/NVM *Dma…
Frauschi Jun 5, 2026
a2e064f
test: exercise client *Dma APIs through a translating DMA callback
Frauschi Jun 5, 2026
f5310d5
Incorporate review findings
Frauschi Jun 9, 2026
089eee2
More review fixes
Frauschi Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 124 additions & 36 deletions src/wh_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1430,46 +1430,52 @@ int wh_Client_KeyCacheDmaRequest(whClientContext* c, uint32_t flags,
const void* keyAddr, uint16_t keySz,
uint16_t keyId)
{
int ret;
whMessageKeystore_CacheDmaRequest* req = NULL;
int ret = WH_ERROR_OK;
whMessageKeystore_CacheDmaRequest* req = NULL;
uintptr_t keyAddrPtr = 0;
uint16_t capSz = 0;

if (c == NULL || (labelSz > 0 && label == NULL)) {
return WH_ERROR_BADARGS;
}
/* Fail fast if busy: don't acquire a mapping a rejected send would leak. */
if (wh_CommClient_IsRequestPending(c->comm) == 1) {
return WH_ERROR_REQUEST_PENDING;
}

req = (whMessageKeystore_CacheDmaRequest*)wh_CommClient_GetDataPtr(c->comm);
if (req == NULL) {
return WH_ERROR_BADARGS;
}
memset(req, 0, sizeof(*req));
req->id = keyId;
req->flags = flags;
req->labelSz = 0;

/* Set up DMA buffer info */
req->key.sz = keySz;
ret = wh_Client_DmaProcessClientAddress(
c, (uintptr_t)keyAddr, (void**)&keyAddrPtr, keySz,
WH_DMA_OPER_CLIENT_READ_PRE, (whDmaFlags){0});
req->key.addr = keyAddrPtr;

/* Copy label if provided, truncate if necessary */
if (labelSz > 0 && label != NULL) {
capSz = (labelSz > WH_NVM_LABEL_LEN) ? WH_NVM_LABEL_LEN : labelSz;
req->labelSz = capSz;
memcpy(req->label, label, capSz);
}

/* PRE-translate the input key buffer and stash it for the Response POST.
* POST runs in the Response, not here: the server reads the buffer between
* request and response, so an in-request POST would free the scratch too
* early (use-after-free). */
ret = wh_Client_DmaAsyncPre(c, &c->dma.asyncCtx.buf, (uintptr_t)keyAddr,
keySz, WH_DMA_OPER_CLIENT_READ_PRE, &keyAddrPtr);
if (ret == WH_ERROR_OK) {
/* Build and send the request now that the buffer is mapped. */
req->id = keyId;
req->flags = flags;
req->key.addr = (uint64_t)keyAddrPtr;
req->key.sz = keySz;
if (labelSz > 0 && label != NULL) {
capSz = (labelSz > WH_NVM_LABEL_LEN) ? WH_NVM_LABEL_LEN : labelSz;
req->labelSz = capSz;
memcpy(req->label, label, capSz);
}

ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_CACHE_DMA,
sizeof(*req), (uint8_t*)req);
}

(void)wh_Client_DmaProcessClientAddress(
c, (uintptr_t)keyAddr, (void**)&keyAddrPtr, keySz,
WH_DMA_OPER_CLIENT_READ_POST, (whDmaFlags){0});
/* On any failure release the mapping; POST no-ops on the unset slot, so a
* failed PRE needs no separate guard. */
if (ret != WH_ERROR_OK) {
(void)wh_Client_DmaAsyncPost(c, &c->dma.asyncCtx.buf);
}
return ret;
}

Expand All @@ -1492,6 +1498,11 @@ int wh_Client_KeyCacheDmaResponse(whClientContext* c, uint16_t* keyId)
}

ret = wh_Client_RecvResponse(c, &group, &action, &size, (uint8_t*)resp);
/* NOTREADY: response not in yet - return without POST so the pending
* request keeps its mapping; POST runs once the response arrives. */
if (ret == WH_ERROR_NOTREADY) {
Comment thread
bigbrett marked this conversation as resolved.
return ret;
}

if (ret == 0) {
/* Validate response */
Expand All @@ -1510,6 +1521,12 @@ int wh_Client_KeyCacheDmaResponse(whClientContext* c, uint16_t* keyId)
}
}
}

/* POST cleanup: release the input mapping the server has finished reading.
* The key is already cached server-side, so failing to release the
* client-side scratch is a cleanup issue, not an operation failure; don't
* override a successful result with it. */
(void)wh_Client_DmaAsyncPost(c, &c->dma.asyncCtx.buf);
return ret;
}

Expand All @@ -1531,23 +1548,44 @@ int wh_Client_KeyCacheDma(whClientContext* c, uint32_t flags, uint8_t* label,
int wh_Client_KeyExportDmaRequest(whClientContext* c, uint16_t keyId,
const void* keyAddr, uint16_t keySz)
{
whMessageKeystore_ExportDmaRequest* req = NULL;
whMessageKeystore_ExportDmaRequest* req = NULL;
uintptr_t keyAddrPtr = 0;
int ret = WH_ERROR_OK;

if (c == NULL || keyId == WH_KEYID_ERASED) {
return WH_ERROR_BADARGS;
}
/* Fail fast if busy: don't acquire a mapping a rejected send would leak. */
if (wh_CommClient_IsRequestPending(c->comm) == 1) {
return WH_ERROR_REQUEST_PENDING;
}

req =
(whMessageKeystore_ExportDmaRequest*)wh_CommClient_GetDataPtr(c->comm);
if (req == NULL) {
return WH_ERROR_BADARGS;
}
req->id = keyId;
req->key.addr = (uint64_t)((uintptr_t)keyAddr);
req->key.sz = keySz;

return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_EXPORT_DMA,
sizeof(*req), (uint8_t*)req);
/* PRE-translate the output key buffer; the server fills it and the
* Response POST copies the result back and releases it. */
ret = wh_Client_DmaAsyncPre(c, &c->dma.asyncCtx.buf, (uintptr_t)keyAddr,
keySz, WH_DMA_OPER_CLIENT_WRITE_PRE,
&keyAddrPtr);
if (ret == WH_ERROR_OK) {
/* Build and send the request now that the buffer is mapped. */
req->id = keyId;
req->key.addr = (uint64_t)keyAddrPtr;
req->key.sz = keySz;

ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_EXPORT_DMA,
sizeof(*req), (uint8_t*)req);
}

/* On any failure release the mapping; POST no-ops on the unset slot. */
if (ret != WH_ERROR_OK) {
(void)wh_Client_DmaAsyncPost(c, &c->dma.asyncCtx.buf);
}
return ret;
}

int wh_Client_KeyExportDmaResponse(whClientContext* c, uint8_t* label,
Expand All @@ -1571,6 +1609,11 @@ int wh_Client_KeyExportDmaResponse(whClientContext* c, uint8_t* label,

rc = wh_Client_RecvResponse(c, &resp_group, &resp_action, &resp_size,
(uint8_t*)resp);
/* NOTREADY: response not in yet - return without POST so the pending
* request keeps its mapping; POST runs once the response arrives. */
if (rc == WH_ERROR_NOTREADY) {
return rc;
}
if (rc == 0) {
/* Validate response */
if ((resp_group != WH_MESSAGE_GROUP_KEY) ||
Expand All @@ -1595,6 +1638,17 @@ int wh_Client_KeyExportDmaResponse(whClientContext* c, uint8_t* label,
}
}
}

/* POST cleanup: copy the exported key back into the caller's buffer and
* release the mapping. This is a WRITE-back: if it fails the caller has no
* valid data, so surface the POST failure over an otherwise-successful
* result. */
{
int postRc = wh_Client_DmaAsyncPost(c, &c->dma.asyncCtx.buf);
if (rc == WH_ERROR_OK) {
rc = postRc;
}
Comment thread
rizlik marked this conversation as resolved.
}
return rc;
}

Expand All @@ -1616,26 +1670,46 @@ int wh_Client_KeyExportPublicDmaRequest(whClientContext* c, whKeyId keyId,
uint16_t algo, void* keyAddr,
uint16_t keySz)
{
whMessageKeystore_ExportPublicDmaRequest* req = NULL;
whMessageKeystore_ExportPublicDmaRequest* req = NULL;
uintptr_t keyAddrPtr = 0;
int ret = WH_ERROR_OK;

if (c == NULL || keyId == WH_KEYID_ERASED) {
return WH_ERROR_BADARGS;
}
/* Fail fast if busy: don't acquire a mapping a rejected send would leak. */
if (wh_CommClient_IsRequestPending(c->comm) == 1) {
return WH_ERROR_REQUEST_PENDING;
}

req =
(whMessageKeystore_ExportPublicDmaRequest*)wh_CommClient_GetDataPtr(
c->comm);
if (req == NULL) {
return WH_ERROR_BADARGS;
}
req->id = keyId;
req->algo = algo;
req->key.addr = (uint64_t)((uintptr_t)keyAddr);
req->key.sz = keySz;

return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY,
WH_KEY_EXPORT_PUBLIC_DMA, sizeof(*req),
(uint8_t*)req);
/* PRE-translate the output public key buffer; see KeyExportDmaRequest. */
ret = wh_Client_DmaAsyncPre(c, &c->dma.asyncCtx.buf, (uintptr_t)keyAddr,
keySz, WH_DMA_OPER_CLIENT_WRITE_PRE,
&keyAddrPtr);
if (ret == WH_ERROR_OK) {
/* Build and send the request now that the buffer is mapped. */
req->id = keyId;
req->algo = algo;
req->key.addr = (uint64_t)keyAddrPtr;
req->key.sz = keySz;

ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY,
WH_KEY_EXPORT_PUBLIC_DMA, sizeof(*req),
(uint8_t*)req);
}

/* On any failure release the mapping; POST no-ops on the unset slot. */
if (ret != WH_ERROR_OK) {
(void)wh_Client_DmaAsyncPost(c, &c->dma.asyncCtx.buf);
}
return ret;
}

int wh_Client_KeyExportPublicDmaResponse(whClientContext* c, uint8_t* label,
Expand All @@ -1660,6 +1734,11 @@ int wh_Client_KeyExportPublicDmaResponse(whClientContext* c, uint8_t* label,

rc = wh_Client_RecvResponse(c, &resp_group, &resp_action, &resp_size,
(uint8_t*)resp);
/* NOTREADY: response not in yet - return without POST so the pending
* request keeps its mapping; POST runs once the response arrives. */
if (rc == WH_ERROR_NOTREADY) {
return rc;
}
if (rc == 0) {
if (resp_size != sizeof(*resp)) {
rc = WH_ERROR_ABORTED;
Expand All @@ -1679,6 +1758,15 @@ int wh_Client_KeyExportPublicDmaResponse(whClientContext* c, uint8_t* label,
}
}
}

/* POST cleanup, a WRITE-back; see KeyExportDmaResponse for why a POST
* failure is surfaced over an otherwise-successful result. */
{
int postRc = wh_Client_DmaAsyncPost(c, &c->dma.asyncCtx.buf);
if (rc == WH_ERROR_OK) {
rc = postRc;
}
}
return rc;
}

Expand Down
78 changes: 77 additions & 1 deletion src/wh_client_dma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,12 @@
int wh_Client_DmaRegisterAllowList(whClientContext* client,
const whDmaAddrAllowList* allowlist)
{
if (NULL == client || NULL == allowlist) {
if (NULL == client) {
return WH_ERROR_BADARGS;
}

/* A NULL allowlist clears any previously registered list (no enforcement),
* symmetric with wh_Client_DmaRegisterCb(NULL). */
client->dma.dmaAddrAllowList = allowlist;

return WH_ERROR_OK;
Expand Down Expand Up @@ -94,4 +96,78 @@ int wh_Client_DmaProcessClientAddress(whClientContext* client,
}
return rc;
}

int wh_Client_DmaAsyncPre(whClientContext* client, whClientDmaAsyncBuf* buf,
uintptr_t clientAddr, uint64_t len, whDmaOper preOper,
uintptr_t* outXformedAddr)
{
int rc = WH_ERROR_OK;
uintptr_t addr = 0;
whDmaOper postOper = WH_DMA_OPER_CLIENT_READ_POST;

if (client == NULL || buf == NULL || outXformedAddr == NULL) {
return WH_ERROR_BADARGS;
}

/* The POST always matches the PRE's direction, so derive it here rather
* than have every caller pass (and risk mismatching) both halves. */
switch (preOper) {
case WH_DMA_OPER_CLIENT_READ_PRE:
postOper = WH_DMA_OPER_CLIENT_READ_POST;
break;
case WH_DMA_OPER_CLIENT_WRITE_PRE:
postOper = WH_DMA_OPER_CLIENT_WRITE_POST;
break;
default:
/* Caller passed a POST or otherwise invalid operation, not a PRE. */
return WH_ERROR_BADARGS;
}

/* Clear the whole slot up front so a skipped or failed PRE leaves nothing
* for the matching POST (run in the Response) to act on. */
memset(buf, 0, sizeof(*buf));
*outXformedAddr = 0;

/* Nothing to map (e.g. an optional buffer that is absent): leave the slot
* cleared and report success with a 0 transformed address. */
if (len == 0) {
return WH_ERROR_OK;
}

rc = wh_Client_DmaProcessClientAddress(client, clientAddr, (void**)&addr,
(size_t)len, preOper,
(whDmaFlags){0});
if (rc == WH_ERROR_OK) {
buf->xformedAddr = addr;
buf->clientAddr = clientAddr;
buf->sz = len;
buf->postOper = postOper;
*outXformedAddr = addr;
}
return rc;
}

int wh_Client_DmaAsyncPost(whClientContext* client, whClientDmaAsyncBuf* buf)
{
int rc;
uintptr_t addr;

if (client == NULL || buf == NULL) {
return WH_ERROR_BADARGS;
}
/* Nothing stashed (PRE was skipped or the slot was already cleaned up):
* a legitimate no-op, distinct from the NULL-arg misuse above. */
if (buf->sz == 0) {
return WH_ERROR_OK;
}

addr = buf->xformedAddr;
rc = wh_Client_DmaProcessClientAddress(client, buf->clientAddr,
(void**)&addr, (size_t)buf->sz,
buf->postOper, (whDmaFlags){0});
/* Clear the whole slot even on failure so a later Response cannot re-run
* the POST; the failure is returned to the caller. */
memset(buf, 0, sizeof(*buf));
return rc;
}
#endif /* WOLFHSM_CFG_DMA */
Loading
Loading