From 55890850841cb8d9ed8236d54aff0f2e994793c3 Mon Sep 17 00:00:00 2001 From: Jonas 'Sortie' Termansen Date: Wed, 13 May 2015 18:49:31 +0200 Subject: [PATCH] Fix unlinkat potentially following paths twice. --- kernel/descriptor.cpp | 24 ++++++++++------------- kernel/include/sortix/kernel/descriptor.h | 3 +-- kernel/initrd.cpp | 4 ++-- kernel/io.cpp | 8 +------- 4 files changed, 14 insertions(+), 25 deletions(-) diff --git a/kernel/descriptor.cpp b/kernel/descriptor.cpp index 3fff557c..9de0f1fe 100644 --- a/kernel/descriptor.cpp +++ b/kernel/descriptor.cpp @@ -606,26 +606,22 @@ int Descriptor::link(ioctx_t* ctx, const char* filename, Ref node) return ret; } -int Descriptor::unlink(ioctx_t* ctx, const char* filename) +int Descriptor::unlinkat(ioctx_t* ctx, const char* filename, int flags) { + if ( flags & ~(AT_REMOVEFILE | AT_REMOVEDIR) ) + return errno = EINVAL, -1; + if ( !(flags & (AT_REMOVEFILE | AT_REMOVEDIR)) ) + flags |= AT_REMOVEFILE; char* final; Ref dir = OpenDirContainingPath(ctx, Ref(this), filename, &final); if ( !dir ) return -1; - int ret = dir->vnode->unlink(ctx, final); - delete[] final; - return ret; -} - -int Descriptor::rmdir(ioctx_t* ctx, const char* filename) -{ - char* final; - Ref dir = OpenDirContainingPath(ctx, Ref(this), - filename, &final); - if ( !dir ) - return -1; - int ret = dir->vnode->rmdir(ctx, final); + int ret = -1; + if ( ret < 0 && (flags & AT_REMOVEFILE) ) + ret = dir->vnode->unlink(ctx, final); + if ( ret < 0 && (flags & AT_REMOVEDIR) ) + ret = dir->vnode->rmdir(ctx, final); delete[] final; return ret; } diff --git a/kernel/include/sortix/kernel/descriptor.h b/kernel/include/sortix/kernel/descriptor.h index 5f06d9b3..96678092 100644 --- a/kernel/include/sortix/kernel/descriptor.h +++ b/kernel/include/sortix/kernel/descriptor.h @@ -80,8 +80,7 @@ public: mode_t mode = 0); int mkdir(ioctx_t* ctx, const char* filename, mode_t mode); int link(ioctx_t* ctx, const char* filename, Ref node); - int unlink(ioctx_t* ctx, const char* filename); - int rmdir(ioctx_t* ctx, const char* filename); + int unlinkat(ioctx_t* ctx, const char* filename, int flags); int symlink(ioctx_t* ctx, const char* oldname, const char* filename); ssize_t readlink(ioctx_t* ctx, char* buf, size_t bufsiz); int tcgetwincurpos(ioctx_t* ctx, struct wincurpos* wcp); diff --git a/kernel/initrd.cpp b/kernel/initrd.cpp index c4508ab6..7c26547c 100644 --- a/kernel/initrd.cpp +++ b/kernel/initrd.cpp @@ -392,13 +392,13 @@ bool ExtractFromPhysicalInto(addr_t physaddr, size_t size, Ref desc) { if ( ((const char*) dirent.d_name)[0] == '.' ) continue; - ctx.links->unlink(&ctx.ioctx, dirent.d_name); + ctx.links->unlinkat(&ctx.ioctx, dirent.d_name, AT_REMOVEFILE); ctx.links->lseek(&ctx.ioctx, 0, SEEK_SET); } ctx.links.Reset(); - desc->rmdir(&ctx.ioctx, ".initrd-links"); + desc->unlinkat(&ctx.ioctx, ".initrd-links", AT_REMOVEDIR); // Unmap the pages and return the physical frames for reallocation. for ( size_t i = 0; i < initrd_addr_alloc.size; i += Page::Size() ) diff --git a/kernel/io.cpp b/kernel/io.cpp index 3d25c851..4e3459c3 100644 --- a/kernel/io.cpp +++ b/kernel/io.cpp @@ -204,8 +204,6 @@ int sys_faccessat(int dirfd, const char* path, int /*mode*/, int flags) int sys_unlinkat(int dirfd, const char* path, int flags) { - if ( !(flags & (AT_REMOVEFILE | AT_REMOVEDIR)) ) - flags |= AT_REMOVEFILE; char* pathcopy = GetStringFromUser(path); if ( !pathcopy ) return -1; @@ -213,11 +211,7 @@ int sys_unlinkat(int dirfd, const char* path, int flags) const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } - int ret = -1; - if ( ret < 0 && (flags & AT_REMOVEFILE) ) - ret = from->unlink(&ctx, relpath); - if ( ret < 0 && (flags & AT_REMOVEDIR) ) - ret = from->rmdir(&ctx, relpath); + int ret = from->unlinkat(&ctx, relpath, flags); delete[] pathcopy; return ret; }