From 4604c86d2fced32c186680788ba98f74df071b23 Mon Sep 17 00:00:00 2001 From: Brian Burkhalter Date: Tue, 3 Jun 2025 15:43:26 +0000 Subject: [PATCH] 8357425: (fs) SecureDirectoryStream setPermissions should use fchmodat Reviewed-by: alanb --- .../sun/nio/fs/UnixNativeDispatcher.java | 22 +++++- .../sun/nio/fs/UnixSecureDirectoryStream.java | 25 ++++--- .../native/libnio/fs/UnixNativeDispatcher.c | 23 +++++++ .../nio/file/DirectoryStream/SecureDS.java | 67 ++++++++++++++++++- 4 files changed, 125 insertions(+), 12 deletions(-) diff --git a/src/java.base/unix/classes/sun/nio/fs/UnixNativeDispatcher.java b/src/java.base/unix/classes/sun/nio/fs/UnixNativeDispatcher.java index 046f843371d..09611f23298 100644 --- a/src/java.base/unix/classes/sun/nio/fs/UnixNativeDispatcher.java +++ b/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"); diff --git a/src/java.base/unix/classes/sun/nio/fs/UnixSecureDirectoryStream.java b/src/java.base/unix/classes/sun/nio/fs/UnixSecureDirectoryStream.java index c583afb588a..5c0693870e6 100644 --- a/src/java.base/unix/classes/sun/nio/fs/UnixSecureDirectoryStream.java +++ b/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(); - try { - fchmod(fd, UnixFileModeAttribute.toUnixMode(perms)); - } catch (UnixException x) { - x.rethrowAsIOException(file); - } finally { - if (file != null && fd >= 0) - UnixNativeDispatcher.close(fd, e-> null); + 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, mode); + } finally { + if (fd >= 0) + UnixNativeDispatcher.close(fd, e-> null); + } } + } catch (UnixException x) { + x.rethrowAsIOException(file); } finally { ds.readLock().unlock(); } diff --git a/src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c b/src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c index fefaac94b48..fbf996886ae 100644 --- a/src/java.base/unix/native/libnio/fs/UnixNativeDispatcher.c +++ b/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) diff --git a/test/jdk/java/nio/file/DirectoryStream/SecureDS.java b/test/jdk/java/nio/file/DirectoryStream/SecureDS.java index e058d782c52..6199596b30c 100644 --- a/test/jdk/java/nio/file/DirectoryStream/SecureDS.java +++ b/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 noperms = EnumSet.noneOf(PosixFilePermission.class); + Set permsDir = getPosixFilePermissions(aDir); + + try (SecureDirectoryStream stream = + (SecureDirectoryStream)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 permsFile = getPosixFilePermissions(file); + Path linkEntry = Path.of("link"); + Path link = createSymbolicLink(aDir.resolve(linkEntry), fileEntry); + Set 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"));