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. Changing the ABI of the existing TA_AVB_CMD_READ_PERSIST_VALUE method to return an error in the case of a too small buffer was rejected. Instead, a new TA_AVB_CMD_READ_PERSIST_VALUE2 method is implemented which does return an error in case of a too small buffer <https://github.com/OP-TEE/optee_os/pull/7959>. Make use of that method, thus making the <bytes> argument to read_pvalue redundant - continue to accept it, but only use it as a hint for the initial size, defaulting to 64. This obviously requires running against an updated op-tee, but as the optee_rpmb command so far has not been usable programmatically (the values read are only printed to the console), no existing boot logic can have been relying on this command. Signed-off-by: Rasmus Villemoes <[email protected]> --- cmd/optee_rpmb.c | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c index 9a3274c4332..6f6c70ac084 100644 --- a/cmd/optee_rpmb.c +++ b/cmd/optee_rpmb.c @@ -72,8 +72,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; @@ -81,6 +81,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()) @@ -93,6 +95,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) { @@ -110,8 +113,16 @@ static int read_persistent_value(const char *name, param[1].u.memref.shm = shm_buf; param[1].u.memref.size = buffer_size; - rc = invoke_func(TA_AVB_CMD_READ_PERSIST_VALUE, + rc = invoke_func(TA_AVB_CMD_READ_PERSIST_VALUE2, 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; @@ -121,8 +132,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); @@ -198,24 +210,23 @@ int do_optee_rpmb_read(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]) { const char *name; - size_t bytes; size_t bytes_read; - void *buffer; + char *buffer = NULL; + size_t bytes = 64; /* Probably enough for most cases to not require two roundtrips. */ 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; + } - 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
