From 940380c986346cd27a96c7f7d9a4d11b29c4617a Mon Sep 17 00:00:00 2001 From: Jesse Buhagiar Date: Thu, 5 Nov 2020 18:12:23 +1100 Subject: [PATCH] Kernel: Prevent `unveil` returning ENOENT with cpath permissions This addresses the issue first enountered in #3644. If a path is first unveiled with "c" permissions, we should NOT return ENOENT if the node does not exist on the disk, as the program will most likely be creating it at a later time. --- Kernel/Syscalls/unveil.cpp | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/Kernel/Syscalls/unveil.cpp b/Kernel/Syscalls/unveil.cpp index dd13d13057..e2da0452cf 100644 --- a/Kernel/Syscalls/unveil.cpp +++ b/Kernel/Syscalls/unveil.cpp @@ -24,6 +24,7 @@ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ +#include #include #include #include @@ -58,18 +59,11 @@ int Process::sys$unveil(Userspace user_params) if (path.value().is_empty() || path.value().characters()[0] != '/') return -EINVAL; - auto custody_or_error = VFS::the().resolve_path_without_veil(path.value(), root_directory()); - if (custody_or_error.is_error()) - // FIXME Should this be EINVAL? - return custody_or_error.error(); - - auto& custody = custody_or_error.value(); - auto new_unveiled_path = custody->absolute_path(); - auto permissions = copy_string_from_user(params.permissions); if (permissions.is_null()) return -EFAULT; + // Let's work out permissions first... unsigned new_permissions = 0; for (const char permission : permissions) { switch (permission) { @@ -90,6 +84,24 @@ int Process::sys$unveil(Userspace user_params) } } + // Now, let's try and resolve the path and obtain custody of the inode on the disk, and if not, bail out with + // the error from resolve_path_without_veil() + // However, if the user specified unveil() with "c" permissions, we don't set errno if ENOENT is encountered, + // because they most likely intend the program to create the file for them later on. + // If this case is encountered, the parent node of the path is returned and the custody of that inode is used instead. + RefPtr parent_custody; // Parent inode in case of ENOENT + String new_unveiled_path; + auto custody_or_error = VFS::the().resolve_path_without_veil(path.value(), root_directory(), &parent_custody); + if (!custody_or_error.is_error()) { + new_unveiled_path = custody_or_error.value()->absolute_path(); + } else if (custody_or_error.error() == -ENOENT && parent_custody && (new_permissions & UnveiledPath::Access::CreateOrRemove)) { + String basename = LexicalPath(path.value()).basename(); + new_unveiled_path = String::formatted("{}/{}", parent_custody->absolute_path(), basename); + } else { + // FIXME Should this be EINVAL? + return custody_or_error.error(); + } + for (auto& unveiled_path : m_unveiled_paths) { if (unveiled_path.path == new_unveiled_path) { if (new_permissions & ~unveiled_path.permissions)