[PATCH] optee/rpmb.c: Fix driver routing memory alignment using a temporary buffer
From: Timothée Cercueil timothee.cercueil@st.com
OP-TEE OS inserts a 6-byte header before a raw RPMB frame which makes RPMB data buffer not 32bit aligned. Many RPMB drivers implicitly expect 32bit alignment of the eMMC frame including arm_pl180_mmci.c, sandbox_mmc.c and stm32_sdmmc2.c
To prevent breaking ABI with OPTEE-OS RPC memrefs, allocate a temporary buffer to copy the data into an aligned memory.
Signed-off-by: Timothée Cercueil timothee.cercueil@st.com Change-Id: Id78d638851a666a35af907ca8de71dae00946457 Signed-off-by: Timothée Cercueil litchi.pi@protonmail.com --- drivers/tee/optee/rpmb.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 0804fc963c..4713df711c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -122,6 +122,10 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req, { struct rpmb_req *sreq = req; struct mmc *mmc; + void *rpmb_data; + void *rpmb_frame_buffer = NULL; + size_t rpmb_data_sz; + int ret;
if (req_size < sizeof(*sreq)) return TEE_ERROR_BAD_PARAMETERS; @@ -131,9 +135,23 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req, mmc = get_mmc(priv, sreq->dev_id); if (!mmc) return TEE_ERROR_ITEM_NOT_FOUND; - if (mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req), - req_size - sizeof(struct rpmb_req), - rsp, rsp_size)) + + rpmb_data = RPMB_REQ_DATA(req); + rpmb_data_sz = req_size - sizeof(struct rpmb_req); + if (!IS_ALIGNED((uintptr_t)rpmb_data, sizeof(u32))) { + /* 32bit data alignment is required by RPMB driver */ + rpmb_frame_buffer = malloc(rpmb_data_sz); + if (!rpmb_frame_buffer) + return TEE_ERROR_OUT_OF_MEMORY; + + memcpy(rpmb_frame_buffer, rpmb_data, rpmb_data_sz); + rpmb_data = rpmb_frame_buffer; + } + + ret = mmc_rpmb_route_frames(mmc, rpmb_data, rpmb_data_sz, + rsp, rsp_size); + free(rpmb_frame_buffer); + if (ret) return TEE_ERROR_BAD_PARAMETERS; return TEE_SUCCESS;
-- 2.17.1
On 08.06.21 12:07, Timothée Cercueil wrote:
From: Timothée Cercueil timothee.cercueil@st.com
OP-TEE OS inserts a 6-byte header before a raw RPMB frame which makes RPMB data buffer not 32bit aligned. Many RPMB drivers implicitly expect 32bit alignment of the eMMC frame including arm_pl180_mmci.c, sandbox_mmc.c and stm32_sdmmc2.c
Generally block device drivers expect ARCH_DMA_MINALIGN. Have a look at mmc_rpmb_read(), mmc_rpmb_write().
Why would we care about alignment in two different modules: drivers/tee/optee/rpmb.c and drivers/mmc/rpmb.c?
Shouldn't the fix be in drivers/mmc/rpmb.c?
Let't include the MMC maintainer in the discussion.
Best regards
Heinrich
To prevent breaking ABI with OPTEE-OS RPC memrefs, allocate a temporary buffer to copy the data into an aligned memory.
Signed-off-by: Timothée Cercueil timothee.cercueil@st.com Change-Id: Id78d638851a666a35af907ca8de71dae00946457 Signed-off-by: Timothée Cercueil litchi.pi@protonmail.com
drivers/tee/optee/rpmb.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 0804fc963c..4713df711c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -122,6 +122,10 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req, { struct rpmb_req *sreq = req; struct mmc *mmc;
void *rpmb_data;
void *rpmb_frame_buffer = NULL;
size_t rpmb_data_sz;
int ret;
if (req_size < sizeof(*sreq)) return TEE_ERROR_BAD_PARAMETERS;
@@ -131,9 +135,23 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req, mmc = get_mmc(priv, sreq->dev_id); if (!mmc) return TEE_ERROR_ITEM_NOT_FOUND;
if (mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req),req_size - sizeof(struct rpmb_req),rsp, rsp_size))
rpmb_data = RPMB_REQ_DATA(req);rpmb_data_sz = req_size - sizeof(struct rpmb_req);if (!IS_ALIGNED((uintptr_t)rpmb_data, sizeof(u32))) {/* 32bit data alignment is required by RPMB driver */rpmb_frame_buffer = malloc(rpmb_data_sz);if (!rpmb_frame_buffer)return TEE_ERROR_OUT_OF_MEMORY;memcpy(rpmb_frame_buffer, rpmb_data, rpmb_data_sz);rpmb_data = rpmb_frame_buffer;}ret = mmc_rpmb_route_frames(mmc, rpmb_data, rpmb_data_sz,rsp, rsp_size);free(rpmb_frame_buffer);if (ret) return TEE_ERROR_BAD_PARAMETERS;-- 2.17.1
I agree, the most generic way to fix this problem would be to ensure memory alignment inside the general RPMB driver. the drivers/tee/optee/rpmb.c driver uses `mmc_rpmb_route_frames`, which is a parallel API used to drive RPMB frames directly into the MMC driver.
I thought that the MMC driver expecting aligned buffer (and panicking if not) was a normal behaviour, and the memory alignment had to be fixed before that. The problem is that it comes from a shared memory allocation from OP-TEE, which is far less easy to fix using `ALLOC_CACHE_ALIGN_BUFFER` or so.
If the MMC driver has to handle unaligned addresses, then the fix should maybe be on the MMC driver itself indeed, Regards,
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Tuesday, June 8, 2021 2:20 PM, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 08.06.21 12:07, Timothée Cercueil wrote:
From: Timothée Cercueil timothee.cercueil@st.com OP-TEE OS inserts a 6-byte header before a raw RPMB frame which makes RPMB data buffer not 32bit aligned. Many RPMB drivers implicitly expect 32bit alignment of the eMMC frame including arm_pl180_mmci.c, sandbox_mmc.c and stm32_sdmmc2.c
Generally block device drivers expect ARCH_DMA_MINALIGN. Have a look at mmc_rpmb_read(), mmc_rpmb_write().
Why would we care about alignment in two different modules: drivers/tee/optee/rpmb.c and drivers/mmc/rpmb.c?
Shouldn't the fix be in drivers/mmc/rpmb.c?
Let't include the MMC maintainer in the discussion.
Best regards
Heinrich
To prevent breaking ABI with OPTEE-OS RPC memrefs, allocate a temporary buffer to copy the data into an aligned memory.
Signed-off-by: Timothée Cercueil timothee.cercueil@st.com Change-Id: Id78d638851a666a35af907ca8de71dae00946457 Signed-off-by: Timothée Cercueil litchi.pi@protonmail.com
drivers/tee/optee/rpmb.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/tee/optee/rpmb.c b/drivers/tee/optee/rpmb.c index 0804fc963c..4713df711c 100644 --- a/drivers/tee/optee/rpmb.c +++ b/drivers/tee/optee/rpmb.c @@ -122,6 +122,10 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req, { struct rpmb_req *sreq = req; struct mmc *mmc;
void *rpmb_data;
void *rpmb_frame_buffer = NULL;
size_t rpmb_data_sz;
int ret; if (req_size < sizeof(*sreq)) return TEE_ERROR_BAD_PARAMETERS; @@ -131,9 +135,23 @@ static u32 rpmb_process_request(struct optee_private *priv, void *req, mmc = get_mmc(priv, sreq->dev_id); if (!mmc) return TEE_ERROR_ITEM_NOT_FOUND;
if (mmc_rpmb_route_frames(mmc, RPMB_REQ_DATA(req),req_size - sizeof(struct rpmb_req),rsp, rsp_size))rpmb_data = RPMB_REQ_DATA(req);rpmb_data_sz = req_size - sizeof(struct rpmb_req);if (!IS_ALIGNED((uintptr_t)rpmb_data, sizeof(u32))) {/* 32bit data alignment is required by RPMB driver */rpmb_frame_buffer = malloc(rpmb_data_sz);if (!rpmb_frame_buffer)return TEE_ERROR_OUT_OF_MEMORY;memcpy(rpmb_frame_buffer, rpmb_data, rpmb_data_sz);rpmb_data = rpmb_frame_buffer;}ret = mmc_rpmb_route_frames(mmc, rpmb_data, rpmb_data_sz,rsp, rsp_size);free(rpmb_frame_buffer);if (ret) return TEE_ERROR_BAD_PARAMETERS; return TEE_SUCCESS;-- 2.17.1
participants (3)
-
Heinrich Schuchardt -
litchi.pi -
Timothée Cercueil