[webkit-reviews] review granted: [Bug 214952] Strip pointers instead of authing for byteOffset to not allow for a possible way to guess data pac : [Attachment 405534] patch

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 214952: Strip pointers instead of authing for byteOffset to not allow for a
possible way to guess data pac
https://bugs.webkit.org/show_bug.cgi?id=214952

Attachment 405534: patch

https://bugs.webkit.org/attachment.cgi?id=405534&action=review




--- Comment #8 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 405534
  --> https://bugs.webkit.org/attachment.cgi?id=405534
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=405534&action=review

r=me.

>>>>>>> Source/JavaScriptCore/ChangeLog:15
>>>>>>> +	 the above because the PAC bits are ignored.
>>>>>> 
>>>>>> Just curious: Why is stripping needed? Won’t both pointers have the same
PAC bits?
>>>>> 
>>>>> One of the pointers (the base) has no PAC bits.
>>>> 
>>>> This isn't true. Ignore me.
>>> 
>>> The real reason is this:
>>> 
>>> The base may be a vector of length M. Signed using M. The view into that
vector has length N, signed using N. N has to be <= M. Therefore, they might
have the same bits, when M == N, but there is no guarantee they do.
>> 
>> In some cases like this I have seen people do arithmetic and then strip the
result, to do half a much stripping. But I have no idea if this is relevant,
acceptable, and valuable enough to be worthwhile here.
> 
> Let me look it up in some CPU docs. My assumption in writing this code is
that stripping is 1-cycle (since it's essentially a masking operation). Even if
it's not, to do the arithmetic, I think we'd need to do an extra load to get to
the number that we need to do arithmetic on.

>From conversations with CPU folks, my understanding is that stripping is
"free". And FWIW, at least on Apple Silicon ™️, it's also "free" to do an auth
as long as the result of that auth doesn't flow (either directly or via a
load/store) into a branch.