It is implied by the comments in avb_ops.h and the translation of TEE_ERROR_STORAGE_NO_SPACE to AVB_IO_RESULT_ERROR_INSUFFICIENT_SPACE done in common/avb_verify.c:invoke_func() that the TA_AVB_CMD_READ_PERSIST_VALUE could return TEE_ERROR_STORAGE_NO_SPACE when the value is longer than the passed buffer size, and that param[1].u.memref.size would be set to the actual size, so that one can allocate an appropriate buffer and re-read.
However, that has AFAICT never been the case; there is no mention of TEE_ERROR_STORAGE_NO_SPACE in the history of ta/avb/ in https://github.com/OP-TEE/optee_os.git, and what the code does instead is to return a value truncated to the given buffer size. In other words, not only can one not determine the correct buffer size to allocate, one is not even told that truncation happened. A fix is proposed on the op-tee side (https://github.com/OP-TEE/optee_os/pull/7959), but the maintainer would like to see at least some callers updated before it can get merged, which makes sense. Signed-off-by: Rasmus Villemoes <[email protected]> --- cmd/optee_rpmb.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c index cc384c7041e..ba10c1d7b82 100644 --- a/cmd/optee_rpmb.c +++ b/cmd/optee_rpmb.c @@ -71,8 +71,8 @@ static int invoke_func(u32 func, ulong num_param, struct tee_param *param) } static int read_persistent_value(const char *name, - size_t buffer_size, - u8 *out_buffer, + size_t size_hint, + char **out_buffer, size_t *out_num_bytes_read) { int rc = 0; @@ -80,6 +80,8 @@ static int read_persistent_value(const char *name, struct tee_shm *shm_buf; struct tee_param param[2]; size_t name_size = strlen(name) + 1; + size_t buffer_size = size_hint; + int retry = 1; if (!tee) if (avb_ta_open_session()) @@ -92,6 +94,7 @@ static int read_persistent_value(const char *name, goto close_session; } +again: rc = tee_shm_alloc(tee, buffer_size, TEE_SHM_ALLOC, &shm_buf); if (rc) { @@ -111,6 +114,14 @@ static int read_persistent_value(const char *name, rc = invoke_func(TA_AVB_CMD_READ_PERSIST_VALUE, 2, param); + + if (rc == -ENOSPC && param[1].u.memref.size > buffer_size && retry) { + retry = 0; + tee_shm_free(shm_buf); + buffer_size = param[1].u.memref.size; + goto again; + } + if (rc) goto out; @@ -120,8 +131,9 @@ static int read_persistent_value(const char *name, } *out_num_bytes_read = param[1].u.memref.size; - - memcpy(out_buffer, shm_buf->addr, *out_num_bytes_read); + *out_buffer = memdup(shm_buf->addr, *out_num_bytes_read); + if (!*out_buffer) + rc = -ENOMEM; out: tee_shm_free(shm_buf); @@ -199,22 +211,24 @@ int do_optee_rpmb_read(struct cmd_tbl *cmdtp, int flag, int argc, const char *name; size_t bytes; size_t bytes_read; - void *buffer; + char *buffer = NULL; char *endp; - if (argc != 3) + /* Use a third argument merely as a size hint. */ + if (argc < 2 || argc > 3) return CMD_RET_USAGE; name = argv[1]; - bytes = dectoul(argv[2], &endp); - if (*endp && *endp != '\n') - return CMD_RET_USAGE; - - buffer = malloc(bytes); - if (!buffer) - return CMD_RET_FAILURE; + if (argc >= 3) { + bytes = dectoul(argv[2], &endp); + if (*endp && *endp != '\n') + return CMD_RET_USAGE; + } else { + /* Probably enough for most cases to not require two roundtrips. */ + bytes = 64; + } - if (read_persistent_value(name, bytes, buffer, &bytes_read) == 0) { + if (read_persistent_value(name, bytes, &buffer, &bytes_read) == 0) { printf("Read %zu bytes, value = %s\n", bytes_read, (char *)buffer); free(buffer); -- 2.55.0
