On Thu, May 12 2022 at 16:23, Peter Zijlstra wrote:
> On Thu, May 12, 2022 at 03:06:38PM +0200, Thomas Gleixner wrote:
>
>> #define untagged_addr(addr) ({ \
>> u64 __addr = (__force u64)(addr); \
>> \
>> __addr &= current->thread.lam_untag_mask; \
>> (__force __typeof__(addr))__addr; \
>> })
>>
>> No conditionals, fast _and_ correct. Setting this untag mask up once
>> when LAM is enabled is not rocket science.
>
> But that goes wrong if someone ever wants to untag a kernel address and
> not use the result for access_ok().
>
> I'd feel better about something like:
>
> s64 __addr = (addr);
> s64 __sign = __addr;
>
> __sign >>= 63;
> __sign &= lam_untag_mask;
that needs to be
__sign &= ~lam_untag_mask;
> __addr &= lam_untag_mask;
> __addr |= __sign;
>
> __addr;
>
> Which simply extends bit 63 downwards -- although possibly there's an
> easier way to do that, this is pretty gross.
For the price of a conditional:
__addr &= lam_untag_mask;
if (__addr & BIT(63))
__addr |= ~lam_untag_mask;
Now you have the choice between gross and ugly.
Thanks,
tglx
