Bug 214952 – Strip pointers instead of authing for byteOffset to not allow for a possible way to guess data pac

• Component: JavaScriptCore
• Status: REOPENED

[Saam Barati]

The byteOffset operation doesn't load, and just returns an int. It's fine to not auth in this scenario.

[Saam Barati]

Created attachment 405534 [details]
patch

[Darin Adler]

Comment on attachment 405534 [details]
patch

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

> Source/JavaScriptCore/ChangeLog:15
> +        Since byteOffset does no loads/stores, it suffices to just strip the PAC
> +        bits before doing the subtraction. This eliminates any such attacks like
> +        the above because the PAC bits are ignored.

Just curious: Why is stripping needed? Won’t both pointers have the same PAC bits?

[Saam Barati]

(In reply to Darin Adler from comment #2)
> Comment on attachment 405534 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=405534&action=review
> 
> > Source/JavaScriptCore/ChangeLog:15
> > +        Since byteOffset does no loads/stores, it suffices to just strip the PAC
> > +        bits before doing the subtraction. This eliminates any such attacks like
> > +        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.

[Saam Barati]

Comment on attachment 405534 [details]
patch

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

>>> 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.

[Saam Barati]

Comment on attachment 405534 [details]
patch

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

>>>> 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.

[Darin Adler]

Comment on attachment 405534 [details]
patch

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

I can almost review, but not quite. The code looks right to me, but I don’t know enough about conventions in this code to be a helpful reviewer.

>>>>> 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.

[Saam Barati]

(In reply to Darin Adler from comment #6)
> Comment on attachment 405534 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=405534&action=review
> 
> I can almost review, but not quite. The code looks right to me, but I don’t
> know enough about conventions in this code to be a helpful reviewer.
> 
> >>>>> 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.

[Keith Miller]

Comment on attachment 405534 [details]
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.

[EWS]

Committed r265097: <https://trac.webkit.org/changeset/265097>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405534 [details].

[Radar WebKit Bug Importer]

<rdar://problem/66348653>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 215065