From 4a87c95e0a2066d79d457b6b3a5514c70f58af62 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Thu, 5 Dec 2024 15:00:50 -0500 Subject: [PATCH] kernel/fs: Handle O_RDONLY | O_TRUNC in the VFS rather than filesystems. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POSIX specification says that the behavior of specifying O_TRUNC with O_RDONLY is "undefined", but the Linux manpages ominously state "On many systems the file is actually truncated." I tested this, and indeed on Linux the file is actually truncated. This doesn't seem like a very sensible behavior, so in this commit it's changed to return B_NOT_ALLOWED (EPERM) if those flags are specified together. The FAT driver already did this, but most other filesystem drivers just checked write access permissions and truncated the file anyway; so this is indeed a behavioral change. Change-Id: If2e76782743ee91d934dc7e0c2f306f37b159a0f Reviewed-on: https://review.haiku-os.org/c/haiku/+/8625 Reviewed-by: waddlesplash Tested-by: Commit checker robot Reviewed-by: Axel Dörfler --- src/add-ons/kernel/file_systems/bfs/Attribute.cpp | 3 +-- src/add-ons/kernel/file_systems/bfs/Inode.cpp | 3 +-- src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp | 3 +-- src/add-ons/kernel/file_systems/btrfs/Attribute.cpp | 3 +-- src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp | 3 +-- src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp | 3 +-- src/add-ons/kernel/file_systems/ext2/Attribute.cpp | 3 +-- src/add-ons/kernel/file_systems/ext2/Inode.cpp | 3 +-- src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp | 3 +-- src/add-ons/kernel/file_systems/fat/kernel_interface.cpp | 3 --- src/add-ons/kernel/file_systems/xfs/kernel_interface.cpp | 6 ++---- src/system/kernel/fs/vfs.cpp | 6 ++++-- 12 files changed, 15 insertions(+), 27 deletions(-) diff --git a/src/add-ons/kernel/file_systems/bfs/Attribute.cpp b/src/add-ons/kernel/file_systems/bfs/Attribute.cpp index 46339da610..205f16cc44 100644 --- a/src/add-ons/kernel/file_systems/bfs/Attribute.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Attribute.cpp @@ -69,8 +69,7 @@ Attribute::CheckAccess(const char* name, int openMode) || !strcmp(name, "size")*/) RETURN_ERROR(B_NOT_ALLOWED); - return fInode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + return fInode->CheckPermissions(open_mode_to_access(openMode)); } diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp b/src/add-ons/kernel/file_systems/bfs/Inode.cpp index b3d3bc9eae..fda397a9d9 100644 --- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp +++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp @@ -2669,8 +2669,7 @@ Inode::Create(Transaction& transaction, Inode* parent, const char* name, return B_NOT_A_DIRECTORY; // we want to open the file, so we should have the rights to do so - if (inode->CheckPermissions(open_mode_to_access(openMode) - | ((openMode & O_TRUNC) != 0 ? W_OK : 0)) != B_OK) + if (inode->CheckPermissions(open_mode_to_access(openMode)) != B_OK) return B_NOT_ALLOWED; if ((openMode & O_TRUNC) != 0) { diff --git a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp index 381293805e..2b680fb999 100644 --- a/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/bfs/kernel_interface.cpp @@ -1368,8 +1368,7 @@ bfs_open(fs_volume* _volume, fs_vnode* _node, int openMode, void** _cookie) if ((openMode & O_DIRECTORY) != 0 && !inode->IsDirectory()) return B_NOT_A_DIRECTORY; - status_t status = inode->CheckPermissions(open_mode_to_access(openMode) - | ((openMode & O_TRUNC) != 0 ? W_OK : 0)); + status_t status = inode->CheckPermissions(open_mode_to_access(openMode)); if (status != B_OK) RETURN_ERROR(status); diff --git a/src/add-ons/kernel/file_systems/btrfs/Attribute.cpp b/src/add-ons/kernel/file_systems/btrfs/Attribute.cpp index 3defc5a4c8..6f1c5627a2 100644 --- a/src/add-ons/kernel/file_systems/btrfs/Attribute.cpp +++ b/src/add-ons/kernel/file_systems/btrfs/Attribute.cpp @@ -49,8 +49,7 @@ Attribute::~Attribute() status_t Attribute::CheckAccess(const char* name, int openMode) { - return fInode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + return fInode->CheckPermissions(open_mode_to_access(openMode)); } diff --git a/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp index a40ea21b00..f08f70fdd3 100644 --- a/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/btrfs/kernel_interface.cpp @@ -541,8 +541,7 @@ btrfs_open(fs_volume* /*_volume*/, fs_vnode* _node, int openMode, if (inode->IsDirectory() && (openMode & O_RWMASK) != 0) return B_IS_A_DIRECTORY; - status_t status = inode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + status_t status = inode->CheckPermissions(open_mode_to_access(openMode)); if (status != B_OK) return status; diff --git a/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp b/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp index cea433b082..c5e3c95765 100644 --- a/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/exfat/kernel_interface.cpp @@ -462,8 +462,7 @@ exfat_open(fs_volume* /*_volume*/, fs_vnode* _node, int openMode, if (inode->IsDirectory() && (openMode & O_RWMASK) != 0) return B_IS_A_DIRECTORY; - status_t status = inode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + status_t status = inode->CheckPermissions(open_mode_to_access(openMode)); if (status != B_OK) return status; diff --git a/src/add-ons/kernel/file_systems/ext2/Attribute.cpp b/src/add-ons/kernel/file_systems/ext2/Attribute.cpp index 713d8b23fc..614487a182 100644 --- a/src/add-ons/kernel/file_systems/ext2/Attribute.cpp +++ b/src/add-ons/kernel/file_systems/ext2/Attribute.cpp @@ -64,8 +64,7 @@ Attribute::InitCheck() status_t Attribute::CheckAccess(const char* name, int openMode) { - return fInode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + return fInode->CheckPermissions(open_mode_to_access(openMode)); } diff --git a/src/add-ons/kernel/file_systems/ext2/Inode.cpp b/src/add-ons/kernel/file_systems/ext2/Inode.cpp index 2c4777142d..1f195ec7eb 100644 --- a/src/add-ons/kernel/file_systems/ext2/Inode.cpp +++ b/src/add-ons/kernel/file_systems/ext2/Inode.cpp @@ -546,8 +546,7 @@ Inode::Create(Transaction& transaction, Inode* parent, const char* name, if ((openMode & O_DIRECTORY) != 0 && !inode->IsDirectory()) return B_NOT_A_DIRECTORY; - if (inode->CheckPermissions(open_mode_to_access(openMode) - | ((openMode & O_TRUNC) != 0 ? W_OK : 0)) != B_OK) + if (inode->CheckPermissions(open_mode_to_access(openMode)) != B_OK) return B_NOT_ALLOWED; if ((openMode & O_TRUNC) != 0) { diff --git a/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp b/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp index 31f8d930ce..6454357797 100644 --- a/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/ext2/kernel_interface.cpp @@ -1148,8 +1148,7 @@ ext2_open(fs_volume* _volume, fs_vnode* _node, int openMode, void** _cookie) if (inode->IsDirectory() && (openMode & O_RWMASK) != 0) return B_IS_A_DIRECTORY; - status_t status = inode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + status_t status = inode->CheckPermissions(open_mode_to_access(openMode)); if (status != B_OK) return status; diff --git a/src/add-ons/kernel/file_systems/fat/kernel_interface.cpp b/src/add-ons/kernel/file_systems/fat/kernel_interface.cpp index bc1c607a0a..bd8c918b17 100644 --- a/src/add-ons/kernel/file_systems/fat/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/fat/kernel_interface.cpp @@ -2177,9 +2177,6 @@ dosfs_open(fs_volume* volume, fs_vnode* vnode, int openMode, void** _cookie) if ((bsdVolume->mnt_flag & MNT_RDONLY) != 0 || (fatNode->de_Attributes & ATTR_READONLY) != 0) openMode = (openMode & ~O_RWMASK) | O_RDONLY; - if ((openMode & O_TRUNC) != 0 && (openMode & O_RWMASK) == O_RDONLY) - return B_NOT_ALLOWED; - status_t status = _dosfs_access(bsdVolume, bsdNode, open_mode_to_access(openMode)); if (status != B_OK) RETURN_ERROR(status); diff --git a/src/add-ons/kernel/file_systems/xfs/kernel_interface.cpp b/src/add-ons/kernel/file_systems/xfs/kernel_interface.cpp index 69d5030f38..49f3f49fd0 100644 --- a/src/add-ons/kernel/file_systems/xfs/kernel_interface.cpp +++ b/src/add-ons/kernel/file_systems/xfs/kernel_interface.cpp @@ -274,8 +274,7 @@ xfs_open(fs_volume * /*_volume*/, fs_vnode *_node, int openMode, if (inode->IsDirectory() && (openMode & O_RWMASK) != 0) return B_IS_A_DIRECTORY; - status_t status = inode->CheckPermissions(open_mode_to_access(openMode) - | (openMode & O_TRUNC ? W_OK : 0)); + status_t status = inode->CheckPermissions(open_mode_to_access(openMode)); if (status != B_OK) return status; @@ -556,8 +555,7 @@ xfs_open_attr(fs_volume *_volume, fs_vnode *_node, const char *name, Inode* inode = (Inode*)_node->private_node; - int accessMode = open_mode_to_access(openMode) | (openMode & O_TRUNC ? W_OK : 0); - status = inode->CheckPermissions(accessMode); + status = inode->CheckPermissions(open_mode_to_access(openMode)); if (status < B_OK) return status; diff --git a/src/system/kernel/fs/vfs.cpp b/src/system/kernel/fs/vfs.cpp index 46fcfbfe3d..542cb7ff7c 100644 --- a/src/system/kernel/fs/vfs.cpp +++ b/src/system/kernel/fs/vfs.cpp @@ -2833,12 +2833,14 @@ get_new_fd(struct fd_ops* ops, struct fs_mount* mount, struct vnode* vnode, // If the vnode is locked, we don't allow creating a new file/directory // file_descriptor for it - if (vnode && vnode->mandatory_locked_by != NULL - && (ops == &sFileOps || ops == &sDirectoryOps)) + if (vnode != NULL && vnode->mandatory_locked_by != NULL + && (ops == &sFileOps || ops == &sDirectoryOps)) return B_BUSY; if ((openMode & O_RDWR) != 0 && (openMode & O_WRONLY) != 0) return B_BAD_VALUE; + if ((openMode & O_RWMASK) == O_RDONLY && (openMode & O_TRUNC) != 0) + return B_NOT_ALLOWED; descriptor = alloc_fd(); if (!descriptor)