Re: [PATCH v3 03/14] x86: Add arch specific kasan functions

[Dave Hansen]

On 4/4/25 06:14, Maciej Wieczor-Retman wrote:
> +static inline const void *__tag_set(const void *addr, u8 tag)
> +{
> +	u64 __addr = (u64)addr & ~__tag_shifted(KASAN_TAG_KERNEL);
> +	return (const void *)(__addr | __tag_shifted(tag));
> +}

This becomes a lot clearer to read if you separate out the casting from
the logical bit manipulation. For instance:

static inline const void *__tag_set(const void *__addr, u8 tag)
{
	u64 addr = (u64)__addr;

	addr &= ~__tag_shifted(KASAN_TAG_KERNEL);
	addr |= __tag_shifted(tag);

	return (const void *)addr;
}

Also, unless there's a good reason for it, you might as well limit the
places you need to use "__".

Now that we can read this, I think it's potentially buggy. If someone
went and changed:

#define KASAN_TAG_KERNEL	0xFF

to, say:

#define KASAN_TAG_KERNEL	0xAB

the '&' would miss clearing bits. It works fine in the arm64 implementation:

	u64 __addr = (u64)addr & ~__tag_shifted(0xff);

because they've hard-coded 0xff. I _think_ that's what you actually want
here. You don't want to mask out KASAN_TAG_KERNEL, you actually want to
mask out *ANYTHING* in those bits.

So the best thing is probably to define a KASAN_TAG_MASK that makes it
clear which are the tag bits.