[PATCH v2 0/4] cmd: optee_rpmb: make it usable by scripts

[PATCH v2 0/4] cmd: optee_rpmb: make it usable by scripts

[Rasmus Villemoes]

This all started by wanting to be able to fetch a "persistent value"
from RPMB and use that in boot logic. I noticed that the "optee_rpmb
read_pvalue" subcommand had two flaws in that regard: No way to
actually store the result in e.g. an environment variable, and having
to know and provide a "large enough" size of the value.

For the former, it's just a matter of letting the caller provide the
name of an environment variable to store the result to in lieu of
printing it.

For the latter, the tee side should have provided a way to know if a
too small buffer was passed, but that was never actually implemented,
and instead a silently truncated value would be returned. That has
been fixed by adding a new variant of the
TA_AVB_CMD_READ_PERSIST_VALUE method which does yield an error along
with the proper size to allocate:
https://github.com/OP-TEE/optee_os/pull/7959 . That is not yet merged,
but I assume it will be soonish; these patches should not be applied
until it is.

v2:

- Drop the part touching common/avb_verify.c, now that the old
  TA_AVB_CMD_READ_PERSIST_VALUE method is not changed. That doesn't
  mean it shouldn't be updated at some point (the header file promises
  behaviour which is not actually implemented), but it is out of scope
  for what I intend to do currently.

- Do make use of the new method in optee_rpmb read_pvalue, and
  implement the "store result to env var".

v1: https://lore.kernel.org/u-boot/20260828105005.200338-1-ravi@prevas.dk/

Rasmus Villemoes (4):
  cmd: optee_rpmb: sanitize TEE_ERROR -> E* translations
  optee_ta_avb.h: add TA_AVB_CMD_READ_PERSIST_VALUE2 define
  cmd: optee_rpmb: allocate large enough buffer when reading persistent
    value
  cmd: optee_rpmb: make it usable by scripts

 cmd/optee_rpmb.c           | 70 ++++++++++++++++++++++++++------------
 include/tee/optee_ta_avb.h | 12 +++++++
 2 files changed, 61 insertions(+), 21 deletions(-)

-- 
2.55.0

[PATCH v2 1/4] cmd: optee_rpmb: sanitize TEE_ERROR -> E* translations

[Rasmus Villemoes]

Do not translate both OUT_OF_MEMORY and STORAGE_NO_SPACE to -ENOSPC;
we have -ENOMEM which is more fitting for the former. OTOH, a new
addition to op-tee returns TEE_ERROR_SHORT_BUFFER in case the provided
buffer is too small to hold the result, so also use -ENOSPC for that.

Similarly, -ENOENT is a better match for ITEM_NOT_FOUND than "some IO
went wrong".

None of the callers currently care about the actual error code, so
this makes no functional change, but we will add special handling of
the -ENOSPC in a later patch.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 cmd/optee_rpmb.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
index b155278ee2a..9a3274c4332 100644
--- a/cmd/optee_rpmb.c
+++ b/cmd/optee_rpmb.c
@@ -52,10 +52,12 @@ static int invoke_func(u32 func, ulong num_param, struct tee_param *param)
 	case TEE_SUCCESS:
 		return 0;
 	case TEE_ERROR_OUT_OF_MEMORY:
+		return -ENOMEM;
 	case TEE_ERROR_STORAGE_NO_SPACE:
+	case TEE_ERROR_SHORT_BUFFER:
 		return -ENOSPC;
 	case TEE_ERROR_ITEM_NOT_FOUND:
-		return -EIO;
+		return -ENOENT;
 	case TEE_ERROR_TARGET_DEAD:
 		/*
 		 * The TA has paniced, close the session to reload the TA
-- 
2.55.0

Re: [PATCH v2 1/4] cmd: optee_rpmb: sanitize TEE_ERROR -> E* translations

[Mattijs Korpershoek]

Hi Rasmus,

Thank you for the patch.

On Thu, Sep 03, 2026 at 23:22, Rasmus Villemoes <ravi@prevas.dk> wrote:

> Do not translate both OUT_OF_MEMORY and STORAGE_NO_SPACE to -ENOSPC;
> we have -ENOMEM which is more fitting for the former. OTOH, a new
> addition to op-tee returns TEE_ERROR_SHORT_BUFFER in case the provided
> buffer is too small to hold the result, so also use -ENOSPC for that.
>
> Similarly, -ENOENT is a better match for ITEM_NOT_FOUND than "some IO
> went wrong".
>
> None of the callers currently care about the actual error code, so
> this makes no functional change, but we will add special handling of
> the -ENOSPC in a later patch.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

[PATCH v2 2/4] optee_ta_avb.h: add TA_AVB_CMD_READ_PERSIST_VALUE2 define

[Rasmus Villemoes]

Add the new TA_AVB_CMD_READ_PERSIST_VALUE2 definition, and synchronize
the comment for the existing TA_AVB_CMD_READ_PERSIST_VALUE with
optee-os' ta/avb/include/ta_avb.h.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 include/tee/optee_ta_avb.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/tee/optee_ta_avb.h b/include/tee/optee_ta_avb.h
index 949875a64cb..d8db6d257bc 100644
--- a/include/tee/optee_ta_avb.h
+++ b/include/tee/optee_ta_avb.h
@@ -47,6 +47,7 @@
 
 /*
  * Reads a persistent value corresponding to the given name.
+ * Only reads as much of the value as will fit in the provided buffer.
  *
  * in	params[0].u.memref:	persistent value name
  * out	params[1].u.memref:	read persistent value buffer
@@ -61,4 +62,15 @@
  */
 #define TA_AVB_CMD_WRITE_PERSIST_VALUE	5
 
+/*
+ * Reads a persistent value corresponding to the given name.
+ * If the provided buffer is smaller than the value, returns
+ * TEE_ERROR_SHORT_BUFFER and sets the output buffer size to the true
+ * size of the value.
+ *
+ * in	params[0].memref:	persistent value name
+ * out	params[1].memref:	read persistent value buffer
+ */
+#define TA_AVB_CMD_READ_PERSIST_VALUE2	6
+
 #endif /* __TA_AVB_H */
-- 
2.55.0

Re: [PATCH v2 2/4] optee_ta_avb.h: add TA_AVB_CMD_READ_PERSIST_VALUE2 define

[Mattijs Korpershoek]

Hi Rasmus,

Thank you for the patch.

On Thu, Sep 03, 2026 at 23:22, Rasmus Villemoes <ravi@prevas.dk> wrote:

> Add the new TA_AVB_CMD_READ_PERSIST_VALUE2 definition, and synchronize
> the comment for the existing TA_AVB_CMD_READ_PERSIST_VALUE with
> optee-os' ta/avb/include/ta_avb.h.
>
> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>

I see that this is not yet merged here:
https://github.com/OP-TEE/optee_os/pull/7959

I hope there won't be any other changes in optee_os, otherwise we will
have to re-update here again.

In any case, looks good:

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

> ---
>  include/tee/optee_ta_avb.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/tee/optee_ta_avb.h b/include/tee/optee_ta_avb.h
> index 949875a64cb..d8db6d257bc 100644
> --- a/include/tee/optee_ta_avb.h
> +++ b/include/tee/optee_ta_avb.h
> @@ -47,6 +47,7 @@
>  
>  /*
>   * Reads a persistent value corresponding to the given name.
> + * Only reads as much of the value as will fit in the provided buffer.
>   *
>   * in	params[0].u.memref:	persistent value name
>   * out	params[1].u.memref:	read persistent value buffer
> @@ -61,4 +62,15 @@
>   */
>  #define TA_AVB_CMD_WRITE_PERSIST_VALUE	5
>  
> +/*
> + * Reads a persistent value corresponding to the given name.
> + * If the provided buffer is smaller than the value, returns
> + * TEE_ERROR_SHORT_BUFFER and sets the output buffer size to the true
> + * size of the value.
> + *
> + * in	params[0].memref:	persistent value name
> + * out	params[1].memref:	read persistent value buffer
> + */
> +#define TA_AVB_CMD_READ_PERSIST_VALUE2	6
> +
>  #endif /* __TA_AVB_H */
> -- 
> 2.55.0

Re: [PATCH v2 2/4] optee_ta_avb.h: add TA_AVB_CMD_READ_PERSIST_VALUE2 define

[Rasmus Villemoes]

On Mon, Sep 07 2026, Mattijs Korpershoek <mkorpershoek@kernel.org> wrote:

> Hi Rasmus,
>
> Thank you for the patch.
>
> On Thu, Sep 03, 2026 at 23:22, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
>> Add the new TA_AVB_CMD_READ_PERSIST_VALUE2 definition, and synchronize
>> the comment for the existing TA_AVB_CMD_READ_PERSIST_VALUE with
>> optee-os' ta/avb/include/ta_avb.h.
>>
>> Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
>
> I see that this is not yet merged here:
> https://github.com/OP-TEE/optee_os/pull/7959
>
> I hope there won't be any other changes in optee_os, otherwise we will
> have to re-update here again.

Indeed. I had hoped/assumed that since Jens gave his Reviewed-by it
would be merged soonish. I'll give it a few more days, then ping to see
if there's more I need to do.

> In any case, looks good:
>
> Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

Thanks,
Rasmus

[PATCH v2 3/4] cmd: optee_rpmb: allocate large enough buffer when reading persistent value

[Rasmus Villemoes]

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 <ravi@prevas.dk>
---
 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

Re: [PATCH v2 3/4] cmd: optee_rpmb: allocate large enough buffer when reading persistent value

[Mattijs Korpershoek]

Hi Rasmus,

Thank you for the patch.

On Thu, Sep 03, 2026 at 23:22, Rasmus Villemoes <ravi@prevas.dk> wrote:

> 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 <ravi@prevas.dk>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>

[PATCH v2 4/4] cmd: optee_rpmb: make it usable by scripts

[Rasmus Villemoes]

Currently, there is no way to programmatically fetch a persistent
value and use the result in the boot logic - the read_pvalue
subcommand always just prints the result to the console.

Since the "bytes" argument is now redundant and optional, we can
repurpose it: If given, instead of printing the result, the value is
stored in the U-Boot environment variable by that name. We do continue
to accept a numeric argument (i.e. anything beginning with a digit)
and use that as a size hint, but don't document that.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 cmd/optee_rpmb.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/cmd/optee_rpmb.c b/cmd/optee_rpmb.c
index 6f6c70ac084..06633758673 100644
--- a/cmd/optee_rpmb.c
+++ b/cmd/optee_rpmb.c
@@ -7,6 +7,7 @@
 #include <env.h>
 #include <errno.h>
 #include <image.h>
+#include <linux/ctype.h>
 #include <malloc.h>
 #include <mmc.h>
 #include <tee.h>
@@ -213,22 +214,36 @@ int do_optee_rpmb_read(struct cmd_tbl *cmdtp, int flag, int argc,
 	size_t bytes_read;
 	char *buffer = NULL;
 	size_t bytes = 64; /* Probably enough for most cases to not require two roundtrips. */
+	const char *varname = NULL;
 	char *endp;
 
-	/* Use a third argument merely as a size hint. */
 	if (argc < 2 || argc > 3)
 		return CMD_RET_USAGE;
 
 	name = argv[1];
 	if (argc >= 3) {
-		bytes = dectoul(argv[2], &endp);
-		if (*endp && *endp != '\n')
-			return CMD_RET_USAGE;
+		/*
+		 * For backward compatibility, a numerical third
+		 * argument is accepted, but merely treated as a size
+		 * hint. A non-numerical argument is the name of an
+		 * environment variable to store the value into.
+		 */
+		if (isdigit(argv[2][0])) {
+			bytes = dectoul(argv[2], &endp);
+			if (*endp && *endp != '\n')
+				return CMD_RET_USAGE;
+		} else {
+			varname = argv[2];
+		}
 	}
 
 	if (read_persistent_value(name, bytes, &buffer, &bytes_read) == 0) {
-		printf("Read %zu bytes, value = %s\n", bytes_read,
-		       (char *)buffer);
+		if (varname) {
+			env_set(varname, buffer);
+		} else {
+			printf("Read %zu bytes, value = %s\n", bytes_read,
+			       (char *)buffer);
+		}
 		free(buffer);
 		return CMD_RET_SUCCESS;
 	}
@@ -289,7 +304,7 @@ static int do_optee_rpmb(struct cmd_tbl *cmdtp, int flag, int argc,
 
 U_BOOT_CMD (
 	optee_rpmb, 29, 0, do_optee_rpmb,
-	"Provides commands for testing secure storage on RPMB on OPTEE",
-	"read_pvalue <name> <bytes> - read a persistent value <name>\n"
+	"Provides commands for accessing secure storage on RPMB on OPTEE",
+	"read_pvalue <name> [<varname>] - read a persistent value <name> [store it to env var <varname>]\n"
 	"optee_rpmb write_pvalue <name> <value> - write a persistent value <name>\n"
 	);
-- 
2.55.0