Re: [PATCH v2 1/4] thunderbolt: property: reject u32 wrap in tb_property_entry_valid()

[Mika Westerberg]

Hi Michael,

I was about to apply these but noticed few stylistic issues so can you fix
those and send v3?

On Wed, Apr 15, 2026 at 08:32:17AM -0400, Michael Bommarito wrote:
> entry->value is u32 and entry->length is u16; the sum is performed in
> u32 and wraps.  A malicious XDomain peer can pick
> value = 0xFFFFFF00, length = 0x100 so the sum 0x100000000 wraps to 0

It's 0xffffff00 (e.g lower case).

Ditto everywhere.

> and passes the > block_len check.  tb_property_parse() then passes
> entry->value to parse_dwdata() as a dword offset into the property
> block, reading attacker-directed memory far past the allocation.
> 
> For TEXT-typed entries with the "deviceid" or "vendorid" keys this
> lands in xd->device_name / xd->vendor_name and is readable back via
> the per-XDomain device_name / vendor_name sysfs attributes; the leak
> is NUL-bounded (kstrdup() stops at the first zero byte) and
> untargeted (the attacker picks a delta, not an absolute address).
> DATA-typed entries are parsed into property->value.data but not
> generically surfaced to userspace.
> 
> Use check_add_overflow() so a wrapped sum is rejected.
> 
> Fixes: e69b6c02b4c3 ("thunderbolt: Add functions for parsing and creating XDomain property blocks")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: Codex:gpt-5-4
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> ---
>  drivers/thunderbolt/property.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thunderbolt/property.c b/drivers/thunderbolt/property.c
> index 50cbfc92fe65..f5ee8f531300 100644
> --- a/drivers/thunderbolt/property.c
> +++ b/drivers/thunderbolt/property.c
> @@ -8,6 +8,7 @@
>   */
>  
>  #include <linux/err.h>
> +#include <linux/overflow.h>
>  #include <linux/slab.h>
>  #include <linux/string.h>
>  #include <linux/uuid.h>
> @@ -52,13 +53,16 @@ static inline void format_dwdata(void *dst, const void *src, size_t dwords)
>  static bool tb_property_entry_valid(const struct tb_property_entry *entry,
>  				  size_t block_len)
>  {
> +	u32 end;
> +
>  	switch (entry->type) {
>  	case TB_PROPERTY_TYPE_DIRECTORY:
>  	case TB_PROPERTY_TYPE_DATA:
>  	case TB_PROPERTY_TYPE_TEXT:
>  		if (entry->length > block_len)
>  			return false;
> -		if (entry->value + entry->length > block_len)
> +		if (check_add_overflow(entry->value, (u32)entry->length, &end) ||
> +		    end > block_len)
>  			return false;
>  		break;
>  
> -- 
> 2.53.0