Skip to content

Dma fixes #329

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

Open
jackctj117 wants to merge 2 commits into
base: main
Choose a base branch
from
+68 −34
Open

Dma fixes #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
55f2e89
DMA POST Processing Skipped
jackctj117 Apr 3, 2026
bc29749
Fix DMA resource leak by ensuring POST is always called after success…
jackctj117 Apr 3, 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
72 changes: 46 additions & 26 deletions src/wh_server_cert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,9 +556,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,

#ifdef WOLFHSM_CFG_DMA
case WH_MESSAGE_CERT_ACTION_ADDTRUSTED_DMA: {
whMessageCert_AddTrustedDmaRequest req = {0};
whMessageCert_SimpleResponse resp = {0};
void* cert_data = NULL;
whMessageCert_AddTrustedDmaRequest req = {0};
whMessageCert_SimpleResponse resp = {0};
void* cert_data = NULL;
int cert_dma_pre_ok = 0;

if (req_size != sizeof(req)) {
/* Request is malformed */
Expand All @@ -574,6 +575,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
if (resp.rc == WH_ERROR_OK) {
cert_dma_pre_ok = 1;
}
}
if (resp.rc == WH_ERROR_OK) {
/* Process the add trusted action */
Expand All @@ -586,9 +590,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
(void)WH_SERVER_NVM_UNLOCK(server);
} /* WH_SERVER_NVM_LOCK() */
}
if (resp.rc == WH_ERROR_OK) {
/* Post-process client address */
resp.rc = wh_Server_DmaProcessClientAddress(
/* Always call POST for successful PRE, regardless of operation
* result */
if (cert_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
}
Comment on lines +593 to 599
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DMA READ_POST return code is discarded via a void cast. If the POST callback fails after the certificate is added successfully, this handler will still return WH_ERROR_OK, potentially leaving DMA/cache state inconsistent. Consider propagating POST errors when resp.rc is still WH_ERROR_OK (or logging POST failures).

Copilot uses AI. Check for mistakes.
Expand All @@ -600,11 +605,12 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
}; break;

case WH_MESSAGE_CERT_ACTION_READTRUSTED_DMA: {
whMessageCert_ReadTrustedDmaRequest req = {0};
whMessageCert_SimpleResponse resp = {0};
void* cert_data = NULL;
whMessageCert_ReadTrustedDmaRequest req = {0};
whMessageCert_SimpleResponse resp = {0};
void* cert_data = NULL;
uint32_t cert_len;
whNvmMetadata meta;
int cert_dma_pre_ok = 0;

if (req_size != sizeof(req)) {
/* Request is malformed */
Expand All @@ -620,6 +626,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
if (resp.rc == WH_ERROR_OK) {
cert_dma_pre_ok = 1;
}
}
if (resp.rc == WH_ERROR_OK) {
/* Check metadata to see if the certificate is non-exportable */
Expand All @@ -641,10 +650,11 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
(void)WH_SERVER_NVM_UNLOCK(server);
} /* WH_SERVER_NVM_LOCK() */
}
if (resp.rc == WH_ERROR_OK) {
/* Post-process client address */
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, cert_len,
/* Always call POST for successful PRE, regardless of operation
* result */
if (cert_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
}
Comment on lines +653 to 659
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DMA WRITE_POST return code is ignored. If the certificate read succeeded but POST fails, the handler will still return success, which may mean the client buffer wasn't correctly finalized (e.g., cache flush). Consider capturing the POST rc and using it when resp.rc is still WH_ERROR_OK (or logging failures if you need to preserve the main rc).

Copilot uses AI. Check for mistakes.

Expand All @@ -655,10 +665,11 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
}; break;

case WH_MESSAGE_CERT_ACTION_VERIFY_DMA: {
whMessageCert_VerifyDmaRequest req = {0};
whMessageCert_VerifyDmaResponse resp = {0};
void* cert_data = NULL;
whKeyId keyId = WH_KEYID_ERASED;
whMessageCert_VerifyDmaRequest req = {0};
whMessageCert_VerifyDmaResponse resp = {0};
void* cert_data = NULL;
whKeyId keyId = WH_KEYID_ERASED;
int cert_dma_pre_ok = 0;

if (req_size != sizeof(req)) {
/* Request is malformed */
Expand All @@ -677,6 +688,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
if (resp.rc == WH_ERROR_OK) {
cert_dma_pre_ok = 1;
}
}
if (resp.rc == WH_ERROR_OK) {
resp.rc = WH_SERVER_NVM_LOCK(server);
Expand All @@ -693,9 +707,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
(void)WH_SERVER_NVM_UNLOCK(server);
} /* WH_SERVER_NVM_LOCK() */
}
if (resp.rc == WH_ERROR_OK) {
/* Post-process client address */
resp.rc = wh_Server_DmaProcessClientAddress(
/* Always call POST for successful PRE, regardless of operation
* result */
if (cert_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
Comment on lines +713 to 715
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DMA READ_POST return code is discarded. If cert verification succeeds but POST fails, the response will still indicate success even though the DMA cleanup step failed. Consider propagating POST errors when resp.rc is still WH_ERROR_OK (or logging POST failures).

Suggested change
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
int post_rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
if ((resp.rc == WH_ERROR_OK) && (post_rc != WH_ERROR_OK)) {
resp.rc = post_rc;
}

Copilot uses AI. Check for mistakes.
}
Expand Down Expand Up @@ -766,9 +781,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
#if defined(WOLFHSM_CFG_DMA)
case WH_MESSAGE_CERT_ACTION_VERIFY_ACERT_DMA: {
/* Acert verify request uses standard cert verify request struct */
whMessageCert_VerifyDmaRequest req = {0};
whMessageCert_SimpleResponse resp = {0};
void* cert_data = NULL;
whMessageCert_VerifyDmaRequest req = {0};
whMessageCert_SimpleResponse resp = {0};
void* cert_data = NULL;
int cert_dma_pre_ok = 0;

if (req_size != sizeof(req)) {
/* Request is malformed */
Expand All @@ -783,6 +799,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
rc = wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
if (rc == WH_ERROR_OK) {
cert_dma_pre_ok = 1;
}
}
if (rc == WH_ERROR_OK) {
/* Process the verify action */
Expand All @@ -805,9 +824,10 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
resp.rc = rc;
}
}
if (rc == WH_ERROR_OK) {
/* Post-process client address */
rc = wh_Server_DmaProcessClientAddress(
/* Always call POST for successful PRE, regardless of operation
* result */
if (cert_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.cert_addr, &cert_data, req.cert_len,
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
}
Comment on lines +827 to 833
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DMA READ_POST return code is discarded. If the verify action succeeds but POST fails, this path will still report success (resp.rc remains WH_ERROR_OK), potentially masking DMA/cache maintenance failures. Consider capturing the POST rc and surfacing it when rc/resp.rc is still WH_ERROR_OK (or logging it).

Copilot uses AI. Check for mistakes.
Expand Down
30 changes: 22 additions & 8 deletions src/wh_server_nvm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,8 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
whMessageNvm_SimpleResponse resp = {0};
void* metadata = NULL;
void* data = NULL;
int metadata_dma_pre_ok = 0;
int data_dma_pre_ok = 0;

if (req_size != sizeof(req)) {
/* Request is malformed */
Expand All @@ -363,11 +365,17 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.metadata_hostaddr, &metadata, sizeof(whNvmMetadata),
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
if (resp.rc == 0) {
metadata_dma_pre_ok = 1;
}
}
if (resp.rc == 0) {
resp.rc = wh_Server_DmaProcessClientAddress(
server, req.data_hostaddr, &data, req.data_len,
WH_DMA_OPER_CLIENT_READ_PRE, (whServerDmaFlags){0});
if (resp.rc == 0) {
data_dma_pre_ok = 1;
}
}
if (resp.rc == 0) {
rc = WH_SERVER_NVM_LOCK(server);
Expand All @@ -381,14 +389,15 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
} /* WH_SERVER_NVM_LOCK() */
resp.rc = rc;
}
if (resp.rc == 0) {
/* perform platform-specific host address processing */
resp.rc = wh_Server_DmaProcessClientAddress(
/* Always call POST for successful PREs, regardless of operation
* result */
if (metadata_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.metadata_hostaddr, &metadata, sizeof(whNvmMetadata),
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
}
if (resp.rc == 0) {
resp.rc = wh_Server_DmaProcessClientAddress(
if (data_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.data_hostaddr, &data, req.data_len,
WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0});
}
Comment on lines +392 to 403
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The return code from the DMA *_POST operation is being discarded. If the main operation succeeds but the POST callback fails (e.g., cache flush/unmap error or allowlist rejection), the handler will still return success, which can leave the client with an inconsistent view of the operation. Consider capturing the POST rc and, if the current resp.rc is still WH_ERROR_OK, propagate the POST failure (or at least log it).

Copilot uses AI. Check for mistakes.
Expand All @@ -405,6 +414,7 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
whNvmMetadata meta = {0};
whNvmSize read_len = 0;
void* data = NULL;
int data_dma_pre_ok = 0;

if (req_size != sizeof(req)) {
/* Request is malformed */
Expand Down Expand Up @@ -441,15 +451,19 @@ int wh_Server_HandleNvmRequest(whServerContext* server,
rc = wh_Server_DmaProcessClientAddress(
server, req.data_hostaddr, &data, req.data_len,
WH_DMA_OPER_CLIENT_WRITE_PRE, (whServerDmaFlags){0});
if (rc == 0) {
data_dma_pre_ok = 1;
}
}
if (rc == 0) {
/* Process the Read action */
rc = wh_Nvm_ReadChecked(server->nvm, req.id, req.offset,
read_len, (uint8_t*)data);
}
if (rc == 0) {
/* perform platform-specific host address processing */
rc = wh_Server_DmaProcessClientAddress(
/* Always call POST for successful PRE, regardless of read
* result */
if (data_dma_pre_ok) {
(void)wh_Server_DmaProcessClientAddress(
server, req.data_hostaddr, &data, req.data_len,
WH_DMA_OPER_CLIENT_WRITE_POST, (whServerDmaFlags){0});
}
Comment on lines +463 to 469
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DMA WRITE_POST return code is ignored. This can cause the server to report success even if the POST step fails, which is especially risky for write operations (e.g., cache maintenance not completed). Consider storing the POST rc and returning it when rc is still WH_ERROR_OK (or logging it if you intentionally want to preserve the read rc).

Copilot uses AI. Check for mistakes.
Expand Down
Loading