[PATCH v2] tlv: Add tlv_bind_soc_uid mapping

[PATCH v2] tlv: Add tlv_bind_soc_uid mapping

[Jonas Rebmann]

Particularly when using secure boot with signed TLVs, it may be required
to issue and sign TLVs for specific units. As typically all units of a
board are compiled to validate TLVs against the same key, a "binding"
mechanism is needed if interchange of TLVs across those units must be
prevented. This mapping binds against the UID of the SoC, rendering a
signed TLV with such a field invalid for all but the one unit.

When generating TLVs that use this mapping, the exact case-sensitive
string representation of the SoC UID must be taken into account.

Add the special mapping tlv_bind_soc_uid that aborts TLV parsing if the
supplied string does not match the SoC UID number.

Include this mapping in barebox_tlv_v1_mappings with tag 0x0024 to make
it available in testing and in other setups using the generic tlv
parsers.

Set up tlv_register_default as a late initcall so that it's loaded after
the SoC UID was initialized.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Changes in v2:
- Switch to using barebox_get_soc_uid and rename and reword everything
  accordingly (serial number -> soc uid)
- Init tlv_register_default as late_initcall instead of device_initcall
- Link to v1: https://lore.barebox.org/barebox/20251112-tlv_bind_serial-v1-1-638cf222553a@pengutronix.de
---
 common/tlv/barebox.c | 18 +++++++++++++++++-
 include/tlv/tlv.h    |  1 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/common/tlv/barebox.c b/common/tlv/barebox.c
index 24de3eeaaa..fdba9fa2a5 100644
--- a/common/tlv/barebox.c
+++ b/common/tlv/barebox.c
@@ -1,8 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-only
 
+#include "barebox-info.h"
 #include <common.h>
 #include <net.h>
 #include <tlv/tlv.h>
+#include <param.h>
+#include <string.h>
+
 
 int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
 {
@@ -16,6 +20,16 @@ int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len,
 	return 0;
 }
 
+int tlv_bind_soc_uid(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
+{
+	char *tlv_serial = basprintf("%.*s", len, val);
+
+	if (streq_ptr(tlv_serial, barebox_get_soc_uid()))
+		return __tlv_format_str(dev, map, len, val) ? 0 : -ENOMEM;
+
+	return -EACCES;
+}
+
 int tlv_handle_eth_address(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
 {
 	int i;
@@ -169,6 +183,8 @@ struct tlv_mapping barebox_tlv_v1_mappings[] = {
 	{ 0x0011, tlv_handle_eth_address, "ethernet-address" },
 	/* A sequence of multiple Ethernet addresses */
 	{ 0x0012, tlv_handle_eth_address_seq, "ethernet-address" },
+	/* Reject TLVs if device serial number string does not match CPU serial */
+	{ 0x0024, tlv_bind_soc_uid, "bound-soc_uid"},
 	{ /* sentintel */ },
 };
 
@@ -212,4 +228,4 @@ static int tlv_register_default(void)
 	}
 	return 0;
 }
-device_initcall(tlv_register_default);
+late_initcall(tlv_register_default);
diff --git a/include/tlv/tlv.h b/include/tlv/tlv.h
index 536f61646c..54e3afed45 100644
--- a/include/tlv/tlv.h
+++ b/include/tlv/tlv.h
@@ -37,6 +37,7 @@ extern int tlv_format_hex(struct tlv_device *dev, struct tlv_mapping *map, u16 l
 extern int tlv_format_mac(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val);
 extern int tlv_format_blob(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val);
 extern int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val);
+extern int tlv_bind_soc_uid(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val);
 extern int tlv_handle_eth_address(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val);
 extern int tlv_handle_eth_address_seq(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val);
 

---
base-commit: bafc52d7dc93accb213271e3e5c267c4335d8cb2
change-id: 20251112-tlv_bind_serial-b8b24a6fd4a0

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

Re: [PATCH v2] tlv: Add tlv_bind_soc_uid mapping

[Sascha Hauer]

On Mon, Nov 17, 2025 at 06:14:06PM +0100, Jonas Rebmann wrote:
> Particularly when using secure boot with signed TLVs, it may be required
> to issue and sign TLVs for specific units. As typically all units of a
> board are compiled to validate TLVs against the same key, a "binding"
> mechanism is needed if interchange of TLVs across those units must be
> prevented. This mapping binds against the UID of the SoC, rendering a
> signed TLV with such a field invalid for all but the one unit.
> 
> When generating TLVs that use this mapping, the exact case-sensitive
> string representation of the SoC UID must be taken into account.

Do we really want to have this case-sensitive? I am not sure we're not
creating problems with this once somebody changes the case for
compatibility with the kernel, it was accidently wrong etc.

> 
> Add the special mapping tlv_bind_soc_uid that aborts TLV parsing if the
> supplied string does not match the SoC UID number.
> 
> Include this mapping in barebox_tlv_v1_mappings with tag 0x0024 to make
> it available in testing and in other setups using the generic tlv
> parsers.
> 
> Set up tlv_register_default as a late initcall so that it's loaded after
> the SoC UID was initialized.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
> Changes in v2:
> - Switch to using barebox_get_soc_uid and rename and reword everything
>   accordingly (serial number -> soc uid)
> - Init tlv_register_default as late_initcall instead of device_initcall
> - Link to v1: https://lore.barebox.org/barebox/20251112-tlv_bind_serial-v1-1-638cf222553a@pengutronix.de
> ---
>  common/tlv/barebox.c | 18 +++++++++++++++++-
>  include/tlv/tlv.h    |  1 +
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/common/tlv/barebox.c b/common/tlv/barebox.c
> index 24de3eeaaa..fdba9fa2a5 100644
> --- a/common/tlv/barebox.c
> +++ b/common/tlv/barebox.c
> @@ -1,8 +1,12 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  
> +#include "barebox-info.h"
>  #include <common.h>
>  #include <net.h>
>  #include <tlv/tlv.h>
> +#include <param.h>
> +#include <string.h>
> +
>  
>  int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
>  {
> @@ -16,6 +20,16 @@ int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len,
>  	return 0;
>  }
>  
> +int tlv_bind_soc_uid(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
> +{
> +	char *tlv_serial = basprintf("%.*s", len, val);

tlv_serial is not freed.

> +
> +	if (streq_ptr(tlv_serial, barebox_get_soc_uid()))
> +		return __tlv_format_str(dev, map, len, val) ? 0 : -ENOMEM;

Why not simply forward the return value __tlv_format_str() instead?
(which is 0 or -ENOMEM anyway).

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2] tlv: Add tlv_bind_soc_uid mapping

[Jonas Rebmann]

Hi Sascha,

On 2025-11-18 09:40, Sascha Hauer wrote:
> On Mon, Nov 17, 2025 at 06:14:06PM +0100, Jonas Rebmann wrote:
>> Particularly when using secure boot with signed TLVs, it may be required
>> to issue and sign TLVs for specific units. As typically all units of a
>> board are compiled to validate TLVs against the same key, a "binding"
>> mechanism is needed if interchange of TLVs across those units must be
>> prevented. This mapping binds against the UID of the SoC, rendering a
>> signed TLV with such a field invalid for all but the one unit.
>>
>> When generating TLVs that use this mapping, the exact case-sensitive
>> string representation of the SoC UID must be taken into account.
> 
> Do we really want to have this case-sensitive? I am not sure we're not
> creating problems with this once somebody changes the case for
> compatibility with the kernel, it was accidently wrong etc.

To me the big question is: What is a SoC UID?

Is it an arbitrary string that happens to be, for many SoCs composed of
[0-9A-F] and efficiently represented in binary in the efuses? Then it
feels a bit surprising to me to compare this 'arbitrary vendor-provided
string' case-insensitively.

But if we consider this an arbitrary block of binary data, typically
looked at in hexadecimal then I suggest we use the raw "bytes"-format I
sent an RFC patch for on Nov 12, and compare to
barebox_get_soc_uid_bin(). I originally wrote that RFC patch for storing
SoC UIDs but had a conversation with Ahmad that led me to view the SoC
UID as an arbitrary string. However now that we have
barebox_get_soc_uid_bin(), I'm tempted to change my mind.

>> Add the special mapping tlv_bind_soc_uid that aborts TLV parsing if the
>> supplied string does not match the SoC UID number.
>>
>> Include this mapping in barebox_tlv_v1_mappings with tag 0x0024 to make
>> it available in testing and in other setups using the generic tlv
>> parsers.
>>
>> Set up tlv_register_default as a late initcall so that it's loaded after
>> the SoC UID was initialized.
>>
>> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
>> ---
>> Changes in v2:
>> - Switch to using barebox_get_soc_uid and rename and reword everything
>>    accordingly (serial number -> soc uid)
>> - Init tlv_register_default as late_initcall instead of device_initcall
>> - Link to v1: https://lore.barebox.org/barebox/20251112-tlv_bind_serial-v1-1-638cf222553a@pengutronix.de
>> ---
>>   common/tlv/barebox.c | 18 +++++++++++++++++-
>>   include/tlv/tlv.h    |  1 +
>>   2 files changed, 18 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/tlv/barebox.c b/common/tlv/barebox.c
>> index 24de3eeaaa..fdba9fa2a5 100644
>> --- a/common/tlv/barebox.c
>> +++ b/common/tlv/barebox.c
>> @@ -1,8 +1,12 @@
>>   // SPDX-License-Identifier: GPL-2.0-only
>>   
>> +#include "barebox-info.h"
>>   #include <common.h>
>>   #include <net.h>
>>   #include <tlv/tlv.h>
>> +#include <param.h>
>> +#include <string.h>
>> +
>>   
>>   int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
>>   {
>> @@ -16,6 +20,16 @@ int tlv_handle_serial(struct tlv_device *dev, struct tlv_mapping *map, u16 len,
>>   	return 0;
>>   }
>>   
>> +int tlv_bind_soc_uid(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
>> +{
>> +	char *tlv_serial = basprintf("%.*s", len, val);
> 
> tlv_serial is not freed.

I'm just doing the same here as all other handlers ("handle"s?) in
common/tlv/barebox.c do. The string representation of the TLV field is
consumed by __tlv_format eventually:

   param->value = buf; /* pass ownership */

So not freeing seems correct here.

>> +
>> +	if (streq_ptr(tlv_serial, barebox_get_soc_uid()))
>> +		return __tlv_format_str(dev, map, len, val) ? 0 : -ENOMEM;
> 
> Why not simply forward the return value __tlv_format_str() instead?
> (which is 0 or -ENOMEM anyway).

Here too I'm doing the same as the other handlers do. __tlv_format with
the underscores returns buf upon success, NULL on error and it seems
right that this is probably (yet not guaranteed to be) ENOMEM then.
Anyway all the handlers return ENOMEM if there's an error in
__tlv_format either because they call via the macro

   #define tlv_format(tlvdev, map, ...) ({ __tlv_format(tlvdev, map, basprintf(__VA_ARGS__)) ? 0 : -ENOMEM; })

Or, as tlv_format_str, check directly:

   int tlv_format_str(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
   {
     return __tlv_format_str(dev, map, len, val) ? 0 : -ENOMEM;
   }

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v2] tlv: Add tlv_bind_soc_uid mapping

[Jonas Rebmann]

Hi again, tiny addition:

On 2025-11-18 10:49, Jonas Rebmann wrote:
> On 2025-11-18 09:40, Sascha Hauer wrote:
> To me the big question is: What is a SoC UID?
> 
> Is it an arbitrary string that happens to be, for many SoCs composed of
> [0-9A-F] and efficiently represented in binary in the efuses? Then it
> feels a bit surprising to me to compare this 'arbitrary vendor-provided
> string' case-insensitively.
> 
> But if we consider this an arbitrary block of binary data, typically
> looked at in hexadecimal then I suggest we use the raw "bytes"-format I
> sent an RFC patch for on Nov 12, and compare to
> barebox_get_soc_uid_bin(). I originally wrote that RFC patch for storing
> SoC UIDs but had a conversation with Ahmad that led me to view the SoC
> UID as an arbitrary string. However now that we have
> barebox_get_soc_uid_bin(), I'm tempted to change my mind.

I did consider changing this for v2 however in your [PATCH v2 1/9]
"introduce SoC UID" you mentioned that "Others even print the binary
data as decimal (qcom).". If we where to use 'raw "bytes"-format' as in
my RFC, the data YAMLs would have hexadecimal representation and I'm not
sure if that could get too confusing. At least we could consider to add
a (mandatory?) YAML-field that specifies the number system.

Regards,
Jonas

-- 
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Re: [PATCH v2] tlv: Add tlv_bind_soc_uid mapping

[Sascha Hauer]

On Tue, Nov 18, 2025 at 10:49:32AM +0100, Jonas Rebmann wrote:
> Hi Sascha,
> 
> On 2025-11-18 09:40, Sascha Hauer wrote:
> > On Mon, Nov 17, 2025 at 06:14:06PM +0100, Jonas Rebmann wrote:
> > > Particularly when using secure boot with signed TLVs, it may be required
> > > to issue and sign TLVs for specific units. As typically all units of a
> > > board are compiled to validate TLVs against the same key, a "binding"
> > > mechanism is needed if interchange of TLVs across those units must be
> > > prevented. This mapping binds against the UID of the SoC, rendering a
> > > signed TLV with such a field invalid for all but the one unit.
> > > 
> > > When generating TLVs that use this mapping, the exact case-sensitive
> > > string representation of the SoC UID must be taken into account.
> > 
> > Do we really want to have this case-sensitive? I am not sure we're not
> > creating problems with this once somebody changes the case for
> > compatibility with the kernel, it was accidently wrong etc.
> 
> To me the big question is: What is a SoC UID?
> 
> Is it an arbitrary string that happens to be, for many SoCs composed of
> [0-9A-F] and efficiently represented in binary in the efuses? Then it
> feels a bit surprising to me to compare this 'arbitrary vendor-provided
> string' case-insensitively.
> 
> But if we consider this an arbitrary block of binary data, typically
> looked at in hexadecimal then I suggest we use the raw "bytes"-format I
> sent an RFC patch for on Nov 12, and compare to
> barebox_get_soc_uid_bin(). I originally wrote that RFC patch for storing
> SoC UIDs but had a conversation with Ahmad that led me to view the SoC
> UID as an arbitrary string. However now that we have
> barebox_get_soc_uid_bin(), I'm tempted to change my mind.

At least all drivers in the Kernel use either [0-9A-F] or [0-9].

> > > +	char *tlv_serial = basprintf("%.*s", len, val);
> > 
> > tlv_serial is not freed.
> 
> I'm just doing the same here as all other handlers ("handle"s?) in
> common/tlv/barebox.c do. The string representation of the TLV field is
> consumed by __tlv_format eventually:
> 
>   param->value = buf; /* pass ownership */
> 
> So not freeing seems correct here.

The string you are referring here is allocated in __tlv_format_str():

const char * __tlv_format_str(struct tlv_device *dev, struct tlv_mapping *map, u16 len, const u8 *val)
{
	return __tlv_format(dev, map, basprintf("%.*s", len, val));
}

tlv_serial is another string. It's not freed and not even passes to tlv
functions.

> 
> > > +
> > > +	if (streq_ptr(tlv_serial, barebox_get_soc_uid()))
> > > +		return __tlv_format_str(dev, map, len, val) ? 0 : -ENOMEM;
> > 
> > Why not simply forward the return value __tlv_format_str() instead?
> > (which is 0 or -ENOMEM anyway).
> 
> Here too I'm doing the same as the other handlers do. __tlv_format with
> the underscores returns buf upon success, NULL on error and it seems
> right that this is probably (yet not guaranteed to be) ENOMEM then.
> Anyway all the handlers return ENOMEM if there's an error in
> __tlv_format either because they call via the macro

Ok, I wrongly looked at tlv_format_str() which returns an int. So I
rephrase my question: Why not use tlv_format_str() instead?

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |