Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property
On Fri, 9 Jun 2023 19:44:32 GMT, Chris Hegarty wrote: > Hi all, > > This pull request contains a backport of commit > [cee5724d](https://github.com/openjdk/jdk/commit/cee5724d09b9ef9bd528fb721b756cb052265e3d) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Chris Hegarty on 9 Jun 2023 and > was reviewed by Roger Riggs, Uwe Schindler and Paul Sandoz. > > Thanks! Thanks for the reviews. - PR Comment: https://git.openjdk.org/jdk21/pull/3#issuecomment-1587529121
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property
On Fri, 9 Jun 2023 19:44:32 GMT, Chris Hegarty wrote: > Hi all, > > This pull request contains a backport of commit > [cee5724d](https://github.com/openjdk/jdk/commit/cee5724d09b9ef9bd528fb721b756cb052265e3d) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Chris Hegarty on 9 Jun 2023 and > was reviewed by Roger Riggs, Uwe Schindler and Paul Sandoz. > > Thanks! @RogerRiggs @PaulSandoz this is a clean backport. Could you please add your review. Thanks. - PR Comment: https://git.openjdk.org/jdk21/pull/3#issuecomment-1586171692
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property
On Fri, 9 Jun 2023 19:44:32 GMT, Chris Hegarty wrote: > Hi all, > > This pull request contains a backport of commit > [cee5724d](https://github.com/openjdk/jdk/commit/cee5724d09b9ef9bd528fb721b756cb052265e3d) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported was authored by Chris Hegarty on 9 Jun 2023 and > was reviewed by Roger Riggs, Uwe Schindler and Paul Sandoz. > > Thanks! Marked as reviewed by uschindler (Author). - PR Review: https://git.openjdk.org/jdk21/pull/3#pullrequestreview-1473013982
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v3]
> A trivial use of the Vector API when run with the security manager and a
> domain that does not grant permissions fails with
> java.security.AccessControlException: access denied
> ("java.util.PropertyPermission"
> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>
> The fix it minimal, as consistent with other system property access in the
> JDK - just access the property while asserting privileged. Note: no explicit
> permission grant to the vector module is required, as it is in the boot
> loader.
>
> This is the only such security manager related issue I see in this code, and
> I have looked.
Chris Hegarty has updated the pull request incrementally with one additional
commit since the last revision:
use loopBound
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14392/files
- new: https://git.openjdk.org/jdk/pull/14392/files/4312aec1..5a4be97f
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=14392=02
- incr: https://webrevs.openjdk.org/?repo=jdk=14392=01-02
Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/14392.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14392/head:pull/14392
PR: https://git.openjdk.org/jdk/pull/14392
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v2]
On Fri, 9 Jun 2023 18:12:36 GMT, Paul Sandoz wrote: >> Chris Hegarty has updated the pull request incrementally with one additional >> commit since the last revision: >> >> add at bug and remove newline > > test/jdk/jdk/incubator/vector/VectorRuns.java line 74: > >> 72: return a.length; >> 73: >> 74: int length = a.length & ~(species.length() - 1); > > Recommend: > Suggestion: > > int length = species.loopBound(a.length); Done, [5a4be97](https://github.com/openjdk/jdk/pull/14392/commits/5a4be97faa822a612dbb9d86cf9b6cf4d88483da) - PR Review Comment: https://git.openjdk.org/jdk/pull/14392#discussion_r1224679125
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v2]
On Fri, 9 Jun 2023 13:18:07 GMT, Chris Hegarty wrote:
>> A trivial use of the Vector API when run with the security manager and a
>> domain that does not grant permissions fails with
>> java.security.AccessControlException: access denied
>> ("java.util.PropertyPermission"
>> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>>
>> The fix it minimal, as consistent with other system property access in the
>> JDK - just access the property while asserting privileged. Note: no explicit
>> permission grant to the vector module is required, as it is in the boot
>> loader.
>>
>> This is the only such security manager related issue I see in this code, and
>> I have looked.
>
> Chris Hegarty has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add at bug and remove newline
Here is our Lucene workaround: https://github.com/apache/lucene/pull/12362
-
PR Comment: https://git.openjdk.org/jdk/pull/14392#issuecomment-1584965386
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v2]
On Fri, 9 Jun 2023 13:18:07 GMT, Chris Hegarty wrote:
>> A trivial use of the Vector API when run with the security manager and a
>> domain that does not grant permissions fails with
>> java.security.AccessControlException: access denied
>> ("java.util.PropertyPermission"
>> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>>
>> The fix it minimal, as consistent with other system property access in the
>> JDK - just access the property while asserting privileged. Note: no explicit
>> permission grant to the vector module is required, as it is in the boot
>> loader.
>>
>> This is the only such security manager related issue I see in this code, and
>> I have looked.
>
> Chris Hegarty has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add at bug and remove newline
Marked as reviewed by psandoz (Reviewer).
test/jdk/jdk/incubator/vector/VectorRuns.java line 74:
> 72: return a.length;
> 73:
> 74: int length = a.length & ~(species.length() - 1);
Recommend:
Suggestion:
int length = species.loopBound(a.length);
-
PR Review: https://git.openjdk.org/jdk/pull/14392#pullrequestreview-1472841843
PR Review Comment: https://git.openjdk.org/jdk/pull/14392#discussion_r1224610480
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property
On Fri, 9 Jun 2023 13:07:27 GMT, Chris Hegarty wrote:
>> A trivial use of the Vector API when run with the security manager and a
>> domain that does not grant permissions fails with
>> java.security.AccessControlException: access denied
>> ("java.util.PropertyPermission"
>> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>>
>> The fix it minimal, as consistent with other system property access in the
>> JDK - just access the property while asserting privileged. Note: no explicit
>> permission grant to the vector module is required, as it is in the boot
>> loader.
>>
>> This is the only such security manager related issue I see in this code, and
>> I have looked.
>
> @PaulSandoz We just ran into this yesterday,
> https://github.com/elastic/elasticsearch/pull/96715. The change here is
> trivial.
Thanks @ChrisHegarty for fixing. Lucene's tests didn't find this, unfortunately!
-
PR Comment: https://git.openjdk.org/jdk/pull/14392#issuecomment-1584832808
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v2]
On Fri, 9 Jun 2023 13:18:07 GMT, Chris Hegarty wrote:
>> A trivial use of the Vector API when run with the security manager and a
>> domain that does not grant permissions fails with
>> java.security.AccessControlException: access denied
>> ("java.util.PropertyPermission"
>> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>>
>> The fix it minimal, as consistent with other system property access in the
>> JDK - just access the property while asserting privileged. Note: no explicit
>> permission grant to the vector module is required, as it is in the boot
>> loader.
>>
>> This is the only such security manager related issue I see in this code, and
>> I have looked.
>
> Chris Hegarty has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add at bug and remove newline
Marked as reviewed by uschindler (Author).
-
PR Review: https://git.openjdk.org/jdk/pull/14392#pullrequestreview-1472554819
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v2]
On Fri, 9 Jun 2023 13:18:07 GMT, Chris Hegarty wrote:
>> A trivial use of the Vector API when run with the security manager and a
>> domain that does not grant permissions fails with
>> java.security.AccessControlException: access denied
>> ("java.util.PropertyPermission"
>> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>>
>> The fix it minimal, as consistent with other system property access in the
>> JDK - just access the property while asserting privileged. Note: no explicit
>> permission grant to the vector module is required, as it is in the boot
>> loader.
>>
>> This is the only such security manager related issue I see in this code, and
>> I have looked.
>
> Chris Hegarty has updated the pull request incrementally with one additional
> commit since the last revision:
>
> add at bug and remove newline
Looks good
-
Marked as reviewed by rriggs (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/14392#pullrequestreview-1472530074
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property [v2]
> A trivial use of the Vector API when run with the security manager and a
> domain that does not grant permissions fails with
> java.security.AccessControlException: access denied
> ("java.util.PropertyPermission"
> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>
> The fix it minimal, as consistent with other system property access in the
> JDK - just access the property while asserting privileged. Note: no explicit
> permission grant to the vector module is required, as it is in the boot
> loader.
>
> This is the only such security manager related issue I see in this code, and
> I have looked.
Chris Hegarty has updated the pull request incrementally with one additional
commit since the last revision:
add at bug and remove newline
-
Changes:
- all: https://git.openjdk.org/jdk/pull/14392/files
- new: https://git.openjdk.org/jdk/pull/14392/files/1e15af5a..4312aec1
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=14392=01
- incr: https://webrevs.openjdk.org/?repo=jdk=14392=00-01
Stats: 2 lines in 2 files changed: 1 ins; 1 del; 0 mod
Patch: https://git.openjdk.org/jdk/pull/14392.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/14392/head:pull/14392
PR: https://git.openjdk.org/jdk/pull/14392
Re: RFR: 8309727: Assert privileges while reading the jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK system property
On Fri, 9 Jun 2023 13:02:18 GMT, Chris Hegarty wrote:
> A trivial use of the Vector API when run with the security manager and a
> domain that does not grant permissions fails with
> java.security.AccessControlException: access denied
> ("java.util.PropertyPermission"
> "jdk.incubator.vector.VECTOR_ACCESS_OOB_CHECK" "read").
>
> The fix it minimal, as consistent with other system property access in the
> JDK - just access the property while asserting privileged. Note: no explicit
> permission grant to the vector module is required, as it is in the boot
> loader.
>
> This is the only such security manager related issue I see in this code, and
> I have looked.
@PaulSandoz We just ran into this yesterday,
https://github.com/elastic/elasticsearch/pull/96715. The change here is trivial.
test/jdk/jdk/incubator/vector/VectorRuns.java line 32:
> 30: * @modules jdk.incubator.vector
> 31: * @run main VectorRuns
> 32: * @run main/othervm/java.security.policy=empty_security.policy VectorRuns
I just added a minimal test here, so as not to otherwise disturb other areas.
This is sufficient to very the fix, and ensure that it does not reoccur.
test/jdk/jdk/incubator/vector/VectorRuns.java line 73:
> 71: return a.length;
> 72:
> 73: int length = a.length & ~(species.length() - 1);
pre existing test issue.
-
PR Comment: https://git.openjdk.org/jdk/pull/14392#issuecomment-1584549575
PR Review Comment: https://git.openjdk.org/jdk/pull/14392#discussion_r1224274145
PR Review Comment: https://git.openjdk.org/jdk/pull/14392#discussion_r1224275043
