8357425: (fs) SecureDirectoryStream setPermissions should use fchmodat

[bplb]

Modify to use the fchmodat(2) system call to set permissions where possible to do so. This fixes the problem presented in the issue description.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8357425: (fs) SecureDirectoryStream setPermissions should use fchmodat (Enhancement - P4)

Reviewers

• Alan Bateman (@AlanBateman - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25534/head:pull/25534
$ git checkout pull/25534

Update a local copy of the PR:
$ git checkout pull/25534
$ git pull https://git.openjdk.org/jdk.git pull/25534/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25534

View PR using the GUI difftool:
$ git pr show -t 25534

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25534.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back bpb! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@bplb This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8357425: (fs) SecureDirectoryStream setPermissions should use fchmodat

Reviewed-by: alanb

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 40 new commits pushed to the master branch:

• 19360a9: 8356949: AArch64: Tighten up template interpreter method entry code
• 061b24d: 8357304: [PPC64] C2: Implement MinV, MaxV and Reduction nodes
• c62223a: 8358215: ProblemList jdk/incubator/vector/PreferredSpeciesTest.java
• ... and 37 more: https://git.openjdk.org/jdk/compare/79aff26c2880922b92863911d8a5a035ba9a1e75...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[bplb]

The jdk_nio tests pass on Linux and macOS in the CI.

[openjdk]

@bplb The following label will be automatically applied to this pull request:

• nio

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (bf2316ed)
• 01: Full - Incremental (66de3e52)
• 00: Full (58cb7c9f)

[AlanBateman]

@MBaesken Do you want to confirm that this will build/run on AIX?

[MBaesken]

Hi Alan, thanks for reaching out.

Do you want to confirm that this will build/run on AIX?
I can confirm that it builds on AIX.
Regarding running, the test that is part of the change it linux/macOS only.
When I add aix to the test (test/jdk/java/nio/file/DirectoryStream/SecureDS.java) I get
TEST RESULT: Failed. Execution failed: main' threw exception: java.lang.AssertionError: SecureDirectoryStream not supported.`
but I guess this is expected currently because the test is linux/mac only ?

[AlanBateman]

When I add aix to the test (test/jdk/java/nio/file/DirectoryStream/SecureDS.java) I get
TEST RESULT: Failed. Execution failed: main' threw exception: java.lang.AssertionError: SecureDirectoryStream not supported.`
but I guess this is expected currently because the test is linux/mac only ?

Thanks for checking. The newDirectoryStream methods return a SecureDirectoryStream on platforms that support all the "at" syscalls (list is in UnixNativeDispatcher.c) so I think it means that it is not supported on AIX because some of the "at" calls don't exist. In that case, once it builds it is okay.

[bplb]

The newDirectoryStream methods return a SecureDirectoryStream on platforms that support all the "at" syscalls (list is in UnixNativeDispatcher.c)

Note that I did not add fchmodat to this list of capabilities for all the "at" syscalls as it appears to be supported at least on all configurations certified for Oracle JDK 24. I am not sure about AIX however.

[bplb]

It is supported on AIX 7.1.0 which dates from September 2010.

[AlanBateman]

Note that I did not add fchmodat to this list of capabilities for all the "at" syscalls as it appears to be supported at least on all configurations certified for Oracle JDK 24. I am not sure about AIX however.

Right, and another option to be to not introduce a new capability but it have it covered by SUPPORTS_OPENAT.

[bplb]

Right, and another option to be to not introduce a new capability but it have it covered by SUPPORTS_OPENAT.

I thought of that but the code might be less clean. I will revisit the idea.

[bplb]

I think it means that it is not supported on AIX because some of the "at" calls don't exist

The pertinent code in UnixNativeDispatcher.c is

    /* supports openat, etc. */
 
    if (my_openat_func != NULL &&  my_fstatat_func != NULL &&
        my_unlinkat_func != NULL && my_renameat_func != NULL &&
        my_fdopendir_func != NULL)
    {
        capabilities |= sun_nio_fs_UnixNativeDispatcher_SUPPORTS_OPENAT;
    }

Maybe the list of "at" syscalls needs to be reexamined and/or made more fine grained (not for this PR)?

[AlanBateman]

If you are doing any more edits then you can change this to use try-with-resources.

[AlanBateman]

Maybe the list of "at" syscalls needs to be reexamined and/or made more fine grained (not for this PR)?

Okay, let's go with what you have for now and we can mull over re-visiting this list. It dates from when the support for the "at" functions varied across operating systems and versions.