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

Mika Westerberg
2026-04-27T05:35:40Z

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@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: Codex:gpt-5-4
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> 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