8357425: (fs) SecureDirectoryStream setPermissions should use fchmodat

Reviewed-by: alanb

src/java.base/unix/classes/sun/nio/fs/UnixNativeDispatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -347,6 +347,18 @@ class UnixNativeDispatcher {
     }
     private static native void fchmod0(int fd, int mode) throws UnixException;
 
+    /**
+     * fchmodat(int fd, const char *path, mode_t mode, int flag)
+     */
+    static void fchmodat(int fd, UnixPath path, int mode, int flag)
+        throws UnixException {
+        try (NativeBuffer buffer = copyToNativeBuffer(path)) {
+            fchmodat0(fd, buffer.address(), mode, flag);
+        }
+    }
+    private static native void fchmodat0(int fd, long pathAddress, int mode, int flag)
+        throws UnixException;
+
     /**
      * futimens(int fildes, const struct timespec times[2])
      */
@@ -558,6 +570,14 @@ class UnixNativeDispatcher {
         return (capabilities & SUPPORTS_XATTR) != 0;
     }
 
+    /**
+     * Supports fchmodat with AT_SYMLINK_NOFOLLOW flag
+     */
+    static boolean fchmodatNoFollowSupported() {
+        return fchmodatNoFollowSupported0();
+    }
+    private static native boolean fchmodatNoFollowSupported0();
+
     private static native int init();
     static {
         jdk.internal.loader.BootLoader.loadLibrary("nio");

src/java.base/unix/classes/sun/nio/fs/UnixSecureDirectoryStream.java

@@ -414,15 +414,24 @@ class UnixSecureDirectoryStream
                 if (!ds.isOpen())
                     throw new ClosedDirectoryStreamException();
 
-                int fd = (file == null) ? dfd : open();
+                int mode = UnixFileModeAttribute.toUnixMode(perms);
+                if (file == null)
+                    fchmod(dfd, mode);
+                else if (followLinks)
+                    fchmodat(dfd, file, mode, 0);
+                else if (fchmodatNoFollowSupported())
+                    fchmodat(dfd, file, mode, AT_SYMLINK_NOFOLLOW);
+                else {
+                    int fd = open();
                     try {
-                    fchmod(fd, UnixFileModeAttribute.toUnixMode(perms));
+                        fchmod(fd, mode);
-                } catch (UnixException x) {
-                    x.rethrowAsIOException(file);
                     } finally {
-                    if (file != null && fd >= 0)
+                        if (fd >= 0)
                             UnixNativeDispatcher.close(fd, e-> null);
                     }
+                }
+            } catch (UnixException x) {
+                x.rethrowAsIOException(file);
             } finally {
                 ds.readLock().unlock();
             }

src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c

@@ -398,6 +398,16 @@ Java_sun_nio_fs_UnixNativeDispatcher_init(JNIEnv* env, jclass this)
     return capabilities;
 }
 
+JNIEXPORT jboolean JNICALL
+Java_sun_nio_fs_UnixNativeDispatcher_fchmodatNoFollowSupported0(JNIEnv* env, jclass this) {
+#if defined(__linux__)
+    // Linux recognizes but does not support the AT_SYMLINK_NOFOLLOW flag
+    return JNI_FALSE;
+#else
+    return JNI_TRUE;
+#endif
+}
+
 JNIEXPORT jbyteArray JNICALL
 Java_sun_nio_fs_UnixNativeDispatcher_getcwd(JNIEnv* env, jclass this) {
     jbyteArray result = NULL;
@@ -797,6 +807,19 @@ Java_sun_nio_fs_UnixNativeDispatcher_fchmod0(JNIEnv* env, jclass this, jint file
     }
 }
 
+JNIEXPORT void JNICALL
+Java_sun_nio_fs_UnixNativeDispatcher_fchmodat0(JNIEnv* env, jclass this,
+    jint fd, jlong pathAddress, jint mode, jint flag)
+{
+    int err;
+    const char* path = (const char*)jlong_to_ptr(pathAddress);
+
+    RESTARTABLE(fchmodat((int)fd, path, (mode_t)mode, (int)flag), err);
+    if (err == -1) {
+        throwUnixException(env, errno);
+    }
+}
+
 JNIEXPORT void JNICALL
 Java_sun_nio_fs_UnixNativeDispatcher_chown0(JNIEnv* env, jclass this,
     jlong pathAddress, jint uid, jint gid)

test/jdk/java/nio/file/DirectoryStream/SecureDS.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -22,10 +22,12 @@
  */
 
 /* @test
- * @bug 4313887 6838333 8343020
+ * @bug 4313887 6838333 8343020 8357425
  * @summary Unit test for java.nio.file.SecureDirectoryStream
  * @requires (os.family == "linux" | os.family == "mac")
- * @library ..
+ * @library .. /test/lib
+ * @build jdk.test.lib.Platform
+ * @run main SecureDS
  */
 
 import java.nio.file.*;
@@ -37,6 +39,8 @@ import java.nio.channels.*;
 import java.io.IOException;
 import java.util.*;
 
+import jdk.test.lib.Platform;
+
 public class SecureDS {
     static boolean supportsSymbolicLinks;
 
@@ -54,6 +58,7 @@ public class SecureDS {
             // run tests
             doBasicTests(dir);
             doMoveTests(dir);
+            doSetPermissions(dir);
             miscTests(dir);
 
         } finally {
@@ -172,6 +177,62 @@ public class SecureDS {
         delete(dir2);
     }
 
+    // Exercise setting permisions on the SecureDirectoryStream's view
+    static void doSetPermissions(Path dir) throws IOException {
+        Path aDir = createDirectory(dir.resolve("dir"));
+
+        Set<PosixFilePermission> noperms = EnumSet.noneOf(PosixFilePermission.class);
+        Set<PosixFilePermission> permsDir = getPosixFilePermissions(aDir);
+
+        try (SecureDirectoryStream<Path> stream =
+             (SecureDirectoryStream<Path>)newDirectoryStream(aDir);) {
+
+            // Test setting permission on directory with no permissions
+            setPosixFilePermissions(aDir, noperms);
+            assertTrue(getPosixFilePermissions(aDir).equals(noperms));
+            PosixFileAttributeView view = stream.getFileAttributeView(PosixFileAttributeView.class);
+            view.setPermissions(permsDir);
+            assertTrue(getPosixFilePermissions(aDir).equals(permsDir));
+
+            if (supportsSymbolicLinks) {
+                // Create a file and a link to the file
+                Path fileEntry = Path.of("file");
+                Path file = createFile(aDir.resolve(fileEntry));
+                Set<PosixFilePermission> permsFile = getPosixFilePermissions(file);
+                Path linkEntry = Path.of("link");
+                Path link = createSymbolicLink(aDir.resolve(linkEntry), fileEntry);
+                Set<PosixFilePermission> permsLink = getPosixFilePermissions(link, NOFOLLOW_LINKS);
+
+                // Test following link to file
+                view = stream.getFileAttributeView(link, PosixFileAttributeView.class);
+                view.setPermissions(noperms);
+                assertTrue(getPosixFilePermissions(file).equals(noperms));
+                assertTrue(getPosixFilePermissions(link, NOFOLLOW_LINKS).equals(permsLink));
+                view.setPermissions(permsFile);
+                assertTrue(getPosixFilePermissions(file).equals(permsFile));
+                assertTrue(getPosixFilePermissions(link, NOFOLLOW_LINKS).equals(permsLink));
+
+                // Symbolic link permissions do not apply on Linux
+                if (!Platform.isLinux()) {
+                    // Test not following link to file
+                    view = stream.getFileAttributeView(link, PosixFileAttributeView.class, NOFOLLOW_LINKS);
+                    view.setPermissions(noperms);
+                    assertTrue(getPosixFilePermissions(file).equals(permsFile));
+                    assertTrue(getPosixFilePermissions(link, NOFOLLOW_LINKS).equals(noperms));
+                    view.setPermissions(permsLink);
+                    assertTrue(getPosixFilePermissions(file).equals(permsFile));
+                    assertTrue(getPosixFilePermissions(link, NOFOLLOW_LINKS).equals(permsLink));
+                }
+
+                delete(link);
+                delete(file);
+            }
+
+            // clean-up
+            delete(aDir);
+        }
+    }
+
     // Exercise SecureDirectoryStream's move method
     static void doMoveTests(Path dir) throws IOException {
         Path dir1 = createDirectory(dir.resolve("dir1"));