diff --git a/libc/include/fcntl.h b/libc/include/fcntl.h index 1b8aad72..dc0e97ea 100644 --- a/libc/include/fcntl.h +++ b/libc/include/fcntl.h @@ -34,6 +34,14 @@ __BEGIN_DECLS +/* The kernel would like to simply deal with one bit for each base access mode, + but using the traditional names O_RDONLY, O_WRONLY and O_RDWR for this would + be weird, so it uses O_READ and O_WRITE bits instead. However, we provide the + traditional names here instead to remain compatible. */ +#define O_RDONLY O_READ +#define O_WRONLY O_WRITE +#define O_RDWR (O_READ | O_WRITE) + /* TODO: F_* missing here */ /* TODO: F_RDLCK, F_UNLCK, F_WRLCK missing here */ diff --git a/sortix/descriptor.cpp b/sortix/descriptor.cpp index 2c56c51d..38cc3a91 100644 --- a/sortix/descriptor.cpp +++ b/sortix/descriptor.cpp @@ -43,6 +43,15 @@ namespace Sortix { +// Flags for the various base modes to open a file in. +const int ACCESS_FLAGS = O_READ | O_WRITE | O_EXEC | O_SEARCH; + +// Flags that only make sense at open time. +const int OPEN_FLAGS = O_CREAT | O_DIRECTORY | O_EXCL | O_TRUNC; + +// Flags that only make sense for descriptors. +const int DESCRIPTOR_FLAGS = O_APPEND; + bool LinkInodeInDir(ioctx_t* ctx, Ref dir, const char* name, Ref inode) { @@ -76,7 +85,7 @@ Ref OpenDirContainingPath(ioctx_t* ctx, Ref from, *finalp = final; return from; } - Ref ret = from->open(ctx, dirpath, O_RDONLY | O_DIRECTORY, 0); + Ref ret = from->open(ctx, dirpath, O_READ | O_DIRECTORY, 0); delete[] dirpath; if ( !ret ) { delete[] final; return Ref(); } *finalp = final; @@ -126,11 +135,16 @@ bool Descriptor::IsSeekable() int Descriptor::sync(ioctx_t* ctx) { + // TODO: Possible denial-of-service attack if someone opens the file without + // that much rights and just syncs it a whole lot and slows down the + // system as a whole. return vnode->sync(ctx); } int Descriptor::stat(ioctx_t* ctx, struct stat* st) { + // TODO: Possible information leak if not O_READ | O_WRITE and the caller + // is told about the file size. return vnode->stat(ctx, st); } @@ -148,11 +162,15 @@ int Descriptor::chown(ioctx_t* ctx, uid_t owner, gid_t group) int Descriptor::truncate(ioctx_t* ctx, off_t length) { if ( length < 0 ) { errno = EINVAL; return -1; } + if ( !(dflags & O_WRITE) ) + return errno = EPERM, -1; return vnode->truncate(ctx, length); } off_t Descriptor::lseek(ioctx_t* ctx, off_t offset, int whence) { + // TODO: Possible information leak to let someone without O_READ | O_WRITE + // seek the file and get information about data holes. if ( !IsSeekable() ) return vnode->lseek(ctx, offset, whence); ScopedLock lock(&curofflock); @@ -179,6 +197,8 @@ off_t Descriptor::lseek(ioctx_t* ctx, off_t offset, int whence) ssize_t Descriptor::read(ioctx_t* ctx, uint8_t* buf, size_t count) { + if ( !(dflags & O_READ) ) + return errno = EPERM, -1; if ( !count ) { return 0; } if ( (size_t) SSIZE_MAX < count ) { count = SSIZE_MAX; } if ( !IsSeekable() ) @@ -193,6 +213,8 @@ ssize_t Descriptor::read(ioctx_t* ctx, uint8_t* buf, size_t count) ssize_t Descriptor::pread(ioctx_t* ctx, uint8_t* buf, size_t count, off_t off) { + if ( !(dflags & O_READ) ) + return errno = EPERM, -1; if ( off < 0 ) { errno = EINVAL; return -1; } if ( !count ) { return 0; } if ( SSIZE_MAX < count ) { count = SSIZE_MAX; } @@ -201,6 +223,8 @@ ssize_t Descriptor::pread(ioctx_t* ctx, uint8_t* buf, size_t count, off_t off) ssize_t Descriptor::write(ioctx_t* ctx, const uint8_t* buf, size_t count) { + if ( !(dflags & O_WRITE) ) + return errno = EPERM, -1; if ( !count ) { return 0; } if ( SSIZE_MAX < count ) { count = SSIZE_MAX; } if ( !IsSeekable() ) @@ -219,6 +243,8 @@ ssize_t Descriptor::write(ioctx_t* ctx, const uint8_t* buf, size_t count) ssize_t Descriptor::pwrite(ioctx_t* ctx, const uint8_t* buf, size_t count, off_t off) { + if ( !(dflags & O_WRITE) ) + return errno = EPERM, -1; if ( off < 0 ) { errno = EINVAL; return -1; } if ( !count ) { return 0; } if ( SSIZE_MAX < count ) { count = SSIZE_MAX; } @@ -238,6 +264,22 @@ int Descriptor::isatty(ioctx_t* ctx) ssize_t Descriptor::readdirents(ioctx_t* ctx, struct kernel_dirent* dirent, size_t size, size_t maxcount) { + // TODO: COMPATIBILITY HACK: Traditionally, you can open a directory with + // O_RDONLY and pass it to fdopendir and then use it, which doesn't + // set the needed O_SEARCH flag! I think some software even do it with + // write permissions! Currently, we just let you search the directory + // if you opened with any of the O_SEARCH, O_READ or O_WRITE flags. + // A better solution would be to make fdopendir try to add the + // O_SEARCH flag to the file descriptor. Or perhaps just recheck the + // permissions to search (execute) the directory manually every time, + // though that is less pure. Unfortunately, POSIX is pretty vague on + // how O_SEARCH should be interpreted and most existing Unix systems + // such as Linux doesn't even have that flag! And how about combining + // it with the O_EXEC flag - POSIX allows that and it makes sense + // because the execute bit on directories control search permission. + if ( !(dflags & (O_SEARCH | O_READ | O_WRITE)) ) + return errno = EPERM, -1; + if ( !maxcount ) { return 0; } if ( SSIZE_MAX < size ) { size = SSIZE_MAX; } if ( size < sizeof(*dirent) ) { errno = EINVAL; return -1; } @@ -268,20 +310,44 @@ ssize_t Descriptor::readdirents(ioctx_t* ctx, struct kernel_dirent* dirent, return ret; } +static bool IsSaneFlagModeCombination(int flags, mode_t /*mode*/) +{ + // It doesn't make sense to pass O_CREAT or O_TRUNC when attempting to open + // a directory. We also reject O_TRUNC | O_DIRECTORY early to prevent + // opening a directory, attempting to truncate it, and then aborting with an + // error because a directory was opened. + if ( (flags & (O_CREAT | O_TRUNC)) && (flags & (O_DIRECTORY)) ) + return errno = EINVAL, false; + + return true; +} + +static bool IsLastPathElement(const char* elem) +{ + while ( !(*elem == '/' || *elem == '\0') ) + elem++; + while ( *elem == '/' ) + elem++; + return *elem == '\0'; +} + Ref Descriptor::open(ioctx_t* ctx, const char* filename, int flags, mode_t mode) { + // Unlike early Unix systems, the empty path does not mean the current + // directory on Sortix, so reject it. if ( !filename[0] ) return errno = ENOENT, Ref(); - // O_DIRECTORY makes us only open directories. It is therefore a logical - // error to provide flags that only makes sense for non-directory. We also - // filter reject them early to prevent O_TRUNC | O_DIRECTORY from opening a - // file, truncating it, and then aborting the open with an error. - if ( (flags & (O_CREAT | O_TRUNC)) && (flags & (O_DIRECTORY)) ) + + // Reject some non-sensical flag combinations early. + if ( !IsSaneFlagModeCombination(flags, mode) ) return errno = EINVAL, Ref(); + Ref desc(this); while ( filename[0] ) { + // Reaching a slash in the path means that the caller intended what came + // before to be a directory, stop the open call if it isn't. if ( filename[0] == '/' ) { if ( !S_ISDIR(desc->type) ) @@ -289,25 +355,30 @@ Ref Descriptor::open(ioctx_t* ctx, const char* filename, int flags, filename++; continue; } + + // Cut out the next path element from the input string. size_t slashpos = strcspn(filename, "/"); - bool lastelem = filename[slashpos] == '\0'; char* elem = String::Substring(filename, 0, slashpos); if ( !elem ) return Ref(); - int open_flags = lastelem ? flags : O_RDONLY; - // Forward O_DIRECTORY so the operation can fail earlier. If it doesn't - // fail and we get a non-directory in return, we'll handle it on exit - // with no consequences (since opening an existing file is harmless). - open_flags &= ~(0 /*| O_DIRECTORY*/); + + // Decide how to open the next element in the path. + bool lastelem = IsLastPathElement(filename); + int open_flags = lastelem ? flags : O_READ | O_DIRECTORY; mode_t open_mode = lastelem ? mode : 0; - // TODO: O_NOFOLLOW. + + // Open the next element in the path. Ref next = desc->open_elem(ctx, elem, open_flags, open_mode); delete[] elem; + if ( !next ) return Ref(); + desc = next; filename += slashpos; } + + // Abort the open if the user wanted a directory but this wasn't. if ( flags & O_DIRECTORY && !S_ISDIR(desc->type) ) return errno = ENOTDIR, Ref(); @@ -322,17 +393,30 @@ Ref Descriptor::open_elem(ioctx_t* ctx, const char* filename, int flags, mode_t mode) { assert(!strchr(filename, '/')); - int next_flags = flags & ~(O_APPEND | O_CLOEXEC); - Ref retvnode = vnode->open(ctx, filename, next_flags, mode); + + // Verify that at least one of the base access modes are being used. + if ( !(flags & ACCESS_FLAGS) ) + return errno = EINVAL, Ref(); + + // Filter away flags that only make sense for descriptors. + int retvnode_flags = flags & ~DESCRIPTOR_FLAGS; + Ref retvnode = vnode->open(ctx, filename, retvnode_flags, mode); if ( !retvnode ) return Ref(); - Ref ret(new Descriptor(retvnode, flags & O_APPEND)); + + // Filter away flags that only made sense at during the open call. + int ret_flags = flags & ~OPEN_FLAGS; + Ref ret(new Descriptor(retvnode, ret_flags)); if ( !ret ) return Ref(); + + // Truncate the file if requested. + // TODO: This is a bit dodgy, should this be moved to the inode open method + // or something? And how should error handling be done here? if ( (flags & O_TRUNC) && S_ISREG(ret->type) ) - if ( (flags & O_ACCMODE) == O_WRONLY || - (flags & O_ACCMODE) == O_RDWR ) + if ( flags & O_WRITE ) ret->truncate(ctx, 0); + return ret; } @@ -420,6 +504,8 @@ int Descriptor::rename_here(ioctx_t* ctx, Ref from, ssize_t Descriptor::readlink(ioctx_t* ctx, char* buf, size_t bufsize) { + if ( !(dflags & O_READ) ) + return errno = EPERM, -1; if ( (size_t) SSIZE_MAX < bufsize ) bufsize = (size_t) SSIZE_MAX; return vnode->readlink(ctx, buf, bufsize); @@ -442,6 +528,8 @@ int Descriptor::gettermmode(ioctx_t* ctx, unsigned* mode) int Descriptor::poll(ioctx_t* ctx, PollNode* node) { + // TODO: Perhaps deny polling against some kind of events if this + // descriptor's dflags would reject doing these operations? return vnode->poll(ctx, node); } diff --git a/sortix/fs/user.cpp b/sortix/fs/user.cpp index aaa787a5..b25e9267 100644 --- a/sortix/fs/user.cpp +++ b/sortix/fs/user.cpp @@ -1207,7 +1207,7 @@ void Init(const char* devpath, Ref slashdev) // created, possibly with a O_MKDIR flag to open. if ( slashdev->mkdir(&ctx, "fs", 0755) < 0 && errno != EEXIST ) PanicF("Could not create a %s/fs directory", devpath); - Ref mpoint = slashdev->open(&ctx, "fs", O_RDWR, 0); + Ref mpoint = slashdev->open(&ctx, "fs", O_READ | O_WRITE, 0); if ( !mpoint ) PanicF("Could not open the %s/fs directory", devpath); Ref mtable = CurrentProcess()->GetMTable(); diff --git a/sortix/include/sortix/fcntl.h b/sortix/include/sortix/fcntl.h index 18c79eba..0c63caaa 100644 --- a/sortix/include/sortix/fcntl.h +++ b/sortix/include/sortix/fcntl.h @@ -29,13 +29,11 @@ __BEGIN_DECLS -#define O_RDONLY 1 -#define O_WRONLY 2 -#define O_RDWR 3 -#define O_ACCMODE (O_RDONLY | O_WRONLY | O_RDWR) -#define O_EXEC 4 -#define O_SEARCH 5 -#define O_LOWERFLAGS 0x7 +// Remember to update the flag classifications at the top of descriptor.cpp if +// you add new flags here. +#define O_READ (1<<0) +#define O_WRITE (1<<1) +#define O_EXEC (1<<2) #define O_APPEND (1<<3) #define O_CLOEXEC (1<<4) #define O_CREAT (1<<5) @@ -43,6 +41,8 @@ __BEGIN_DECLS #define O_EXCL (1<<7) #define O_TRUNC (1<<8) #define O_CLOFORK (1<<9) +#define O_SEARCH (1<<10) +// TODO: O_NOFOLLOW. #define FD_CLOEXEC (1<<0) #define FD_CLOFORK (1<<1) diff --git a/sortix/initrd.cpp b/sortix/initrd.cpp index 5adefab1..851021c9 100644 --- a/sortix/initrd.cpp +++ b/sortix/initrd.cpp @@ -276,7 +276,7 @@ static bool ExtractDir(ioctx_t* ctx, uint32_t ino, Ref dir) { if ( dir->mkdir(ctx, name, mode) && errno != EEXIST ) return false; - Ref desc = dir->open(ctx, name, O_RDWR | O_DIRECTORY, 0); + Ref desc = dir->open(ctx, name, O_SEARCH | O_DIRECTORY, 0); if ( !desc ) return false; if ( !ExtractNode(ctx, childino, desc) ) @@ -284,7 +284,7 @@ static bool ExtractDir(ioctx_t* ctx, uint32_t ino, Ref dir) } if ( INITRD_S_ISREG(child->mode) ) { - Ref desc = dir->open(ctx, name, O_WRONLY | O_CREAT, mode); + Ref desc = dir->open(ctx, name, O_WRITE | O_CREAT, mode); if ( !desc ) return false; if ( !ExtractNode(ctx, childino, desc) ) diff --git a/sortix/io.cpp b/sortix/io.cpp index 96ee9067..45b62fb2 100644 --- a/sortix/io.cpp +++ b/sortix/io.cpp @@ -172,7 +172,7 @@ static int sys_faccessat(int dirfd, const char* path, int /*mode*/, int flags) const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } - Ref desc = from->open(&ctx, relpath, O_RDONLY); + Ref desc = from->open(&ctx, relpath, O_READ); delete[] pathcopy; return desc ? 0 : -1; } @@ -244,7 +244,7 @@ static int sys_truncateat(int dirfd, const char* path, off_t length) const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } - Ref desc = from->open(&ctx, relpath, O_WRONLY); + Ref desc = from->open(&ctx, relpath, O_WRITE); delete[] pathcopy; if ( !desc ) return -1; @@ -275,7 +275,7 @@ static int sys_fstatat(int dirfd, const char* path, struct stat* st, int /*flags const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } - Ref desc = from->open(&ctx, relpath, O_RDONLY); + Ref desc = from->open(&ctx, relpath, O_READ); delete[] pathcopy; if ( !desc ) return -1; @@ -353,7 +353,7 @@ static int sys_chdir(const char* path) ioctx_t ctx; SetupUserIOCtx(&ctx); const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath); - Ref desc = from->open(&ctx, relpath, O_RDONLY | O_DIRECTORY); + Ref desc = from->open(&ctx, relpath, O_READ | O_DIRECTORY); from.Reset(); delete[] pathcopy; if ( !desc ) @@ -382,7 +382,7 @@ static int sys_fchownat(int dirfd, const char* path, uid_t owner, gid_t group, i const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } - Ref desc = from->open(&ctx, relpath, O_WRONLY); + Ref desc = from->open(&ctx, relpath, O_WRITE); from.Reset(); delete[] pathcopy; if ( !desc ) @@ -416,7 +416,7 @@ static int sys_fchmodat(int dirfd, const char* path, mode_t mode, int flags) const char* relpath = pathcopy; Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } - Ref desc = from->open(&ctx, relpath, O_WRONLY); + Ref desc = from->open(&ctx, relpath, O_WRITE); from.Reset(); delete[] pathcopy; if ( !desc ) @@ -459,7 +459,7 @@ static int sys_linkat(int olddirfd, const char* oldpath, Ref oldfrom = PrepareLookup(&oldrelpath, olddirfd); if ( !oldfrom ) { delete[] oldpathcopy; delete[] final_elem; return -1; } - Ref file = oldfrom->open(&ctx, oldrelpath, O_RDONLY); + Ref file = oldfrom->open(&ctx, oldrelpath, O_READ); delete[] oldpathcopy; if ( !file ) { delete[] final_elem; return -1; } @@ -552,7 +552,7 @@ static ssize_t sys_readlinkat(int dirfd, const char* path, char* buf, size_t siz Ref from = PrepareLookup(&relpath, dirfd); if ( !from ) { delete[] pathcopy; return -1; } // TODO: Open the symbolic link, instead of what it points to! - Ref desc = from->open(&ctx, relpath, O_RDONLY); + Ref desc = from->open(&ctx, relpath, O_READ); delete[] pathcopy; if ( !desc ) return -1; diff --git a/sortix/kernel.cpp b/sortix/kernel.cpp index 1fae8509..f944d5d2 100644 --- a/sortix/kernel.cpp +++ b/sortix/kernel.cpp @@ -355,7 +355,7 @@ static void BootThread(void* /*user*/) Ref vroot(new Vnode(iroot, Ref(NULL), 0, 0)); if ( !vroot ) Panic("Unable to allocate root vnode."); - Ref droot(new Descriptor(vroot, 0)); + Ref droot(new Descriptor(vroot, O_SEARCH)); if ( !droot ) Panic("Unable to allocate root descriptor."); CurrentProcess()->BootstrapDirectories(droot); @@ -376,7 +376,7 @@ static void BootThread(void* /*user*/) // Get a descriptor for the /dev directory so we can populate it. if ( droot->mkdir(&ctx, "dev", 0775) != 0 && errno != EEXIST ) Panic("Unable to create RAM filesystem /dev directory."); - Ref slashdev = droot->open(&ctx, "dev", O_RDONLY | O_DIRECTORY); + Ref slashdev = droot->open(&ctx, "dev", O_READ | O_DIRECTORY); if ( !slashdev ) Panic("Unable to create descriptor for RAM filesystem /dev directory."); @@ -499,7 +499,7 @@ static void InitThread(void* /*user*/) ioctx_t ctx; SetupKernelIOCtx(&ctx); Ref root = CurrentProcess()->GetRoot(); - Ref init = root->open(&ctx, initpath, O_EXEC); + Ref init = root->open(&ctx, initpath, O_EXEC | O_READ); if ( !init ) PanicF("Could not open %s in early kernel RAM filesystem:\n%s", initpath, strerror(errno)); diff --git a/sortix/pipe.cpp b/sortix/pipe.cpp index 1bec9dfb..1bd3a891 100644 --- a/sortix/pipe.cpp +++ b/sortix/pipe.cpp @@ -26,6 +26,7 @@ #include #include +#include #include #include #include @@ -314,8 +315,8 @@ static int sys_pipe(int pipefd[2]) Ref send_vnode(new Vnode(send_inode, Ref(NULL), 0, 0)); if ( !recv_vnode || !send_vnode ) return -1; - Ref recv_desc(new Descriptor(recv_vnode, 0)); - Ref send_desc(new Descriptor(send_vnode, 0)); + Ref recv_desc(new Descriptor(recv_vnode, O_READ)); + Ref send_desc(new Descriptor(send_vnode, O_WRITE)); if ( !recv_desc || !send_desc ) return -1; Ref dtable = process->GetDTable(); diff --git a/sortix/process.cpp b/sortix/process.cpp index 48069629..c8cfb538 100644 --- a/sortix/process.cpp +++ b/sortix/process.cpp @@ -725,7 +725,7 @@ namespace Sortix SetupKernelIOCtx(&ctx); // TODO: Somehow mark the executable as busy and don't permit writes? - desc = process->Open(&ctx, filename, O_RDONLY, 0); + desc = process->Open(&ctx, filename, O_READ | O_WRITE, 0); if ( !desc ) { goto cleanup_envp; } if ( desc->stat(&ctx, &st) ) { goto cleanup_desc; }