diff --git a/docs/architecture/fat.md b/docs/architecture/fat.md index 5d91ce3..217c9bb 100644 --- a/docs/architecture/fat.md +++ b/docs/architecture/fat.md @@ -92,9 +92,6 @@ directory ends at the first entry whose first byte is zero. ## What it does not do -- **Names longer than 29 bytes.** FAT allows 255 UTF-16 units, but the VFS's - `DIRSIZ` caps a path component, so a longer name is cut short on the way in. - Lifting that is a VFS change. - **Permissions and owners.** Every file shows as `0755 root:root`, like Linux vfat with its defaults. The read-only attribute is kept but not enforced. - **exFAT.** Large SD cards (SDXC, over 32 GB) ship with it. It is a different diff --git a/docs/architecture/filesystem.md b/docs/architecture/filesystem.md index d58ac7c..f90f31b 100644 --- a/docs/architecture/filesystem.md +++ b/docs/architecture/filesystem.md @@ -288,6 +288,20 @@ held alive by an `nlink` bump. Renaming a *directory* is still refused with `EISDIR`, and so is replacing one: both need the `..` fixup and the ancestor-loop check that are not written. +## Names and paths + +A name is at most 255 bytes and a path 4096 including its NUL, Linux's +`NAME_MAX` and `PATH_MAX` (`VFS_NAME_MAX` and `VFS_PATH_MAX` in +`include/file.h`). Longer is `ENAMETOOLONG`. A filesystem may hold less: the v1 +format stores `DIRSIZ` bytes and refuses longer names rather than matching on +their first 30. + +Both limits used to be the copy buffers: every component went through a +`DIRSIZ` array and every path through a `MAXPATH` one, so a name was cut at 29 +bytes and a path at 127, silently. Two files sharing their first 29 bytes were +one file. The path buffers are a page each now, from `kalloc()`, since two of +them would be most of a kernel stack. + ## Symlinks Followed the way Linux follows them, in `namex_from()`. A link in the middle of diff --git a/docs/architecture/schfs.md b/docs/architecture/schfs.md index c1ea7c3..02eac53 100644 --- a/docs/architecture/schfs.md +++ b/docs/architecture/schfs.md @@ -176,7 +176,9 @@ mount. It is set where it is because the page-pointer vector lives inside These are VFS and driver limits rather than SchFS ones: -- `MAXPATH` caps a path at **128 bytes**. +- A path is at most **4096 bytes** and a name **255**, Linux's `PATH_MAX` and + `NAME_MAX` (`VFS_PATH_MAX`, `VFS_NAME_MAX`). Longer is `ENAMETOOLONG`. Both + used to be cut short without a word, at 128 and 29. - `NMOUNT` allows **8** mounts. - The IDE driver handles the primary channel only: **2** ATA devices. - The IDE driver uses LBA48 past 128 GiB, so the disk is not the limit there. diff --git a/include/file.h b/include/file.h index d4f213d..d74dd05 100644 --- a/include/file.h +++ b/include/file.h @@ -171,6 +171,16 @@ struct inode { // Mounted FAT volumes, FAT_DEV onward. See include/fat.h. +// Linux's limits, for every filesystem behind the VFS. A path component can be +// NAME_MAX bytes and a whole path PATH_MAX including its NUL. Each filesystem +// may refuse less: SchFS and FAT take 255, the v1 format DIRSIZ. +// +// The VFS used to copy every component into a DIRSIZ buffer and every path +// into a MAXPATH one, which cut names at 29 bytes and paths at 127 without a +// word -- so a longer name opened whichever file shared its first 29 bytes. +#define VFS_NAME_MAX 255 +#define VFS_PATH_MAX 4096 // one page: namex_from() kalloc()s it + // 1 if some inode on dev with this number is referenced. In fs.c. int iheld(uint dev, uint64 inum); // 1 if any inode on dev but `except` is referenced (0: no exception). In fs.c. diff --git a/kernel/fs/fat/fat.c b/kernel/fs/fat/fat.c index 345d9fc..07dffce 100644 --- a/kernel/fs/fat/fat.c +++ b/kernel/fs/fat/fat.c @@ -667,7 +667,7 @@ static int vfs_name(const char *name, uint16 *out) { int n = 0; - while (n < DIRSIZ && name[n]) + while (n < VFS_NAME_MAX && name[n]) n++; int len = utf8_to_16(name, n, out); if (len < 0) diff --git a/kernel/fs/fs.c b/kernel/fs/fs.c index c75207b..e143cca 100644 --- a/kernel/fs/fs.c +++ b/kernel/fs/fs.c @@ -1345,6 +1345,10 @@ v1_dirlookup(struct inode *dp, char *name, uint *poff) return 0; } + // The format stores DIRSIZ bytes of a name. A longer one cannot be here, + // and comparing only its first DIRSIZ would find a different file. + if (strlen(name) > DIRSIZ) + return 0; tree_search(dp, name, poff, &de, 0); if (de.inum) return iget(dp->dev, de.inum); @@ -1358,6 +1362,8 @@ v1_dirlink(struct inode *dp, char *name, uint64 inum) struct dirent de; struct inode *ip; + if (strlen(name) > DIRSIZ) + return -1; // see v1_dirlookup if ((ip = dirlookup(dp, name, 0)) != 0) { iput(ip); return -1; // already exists @@ -1537,8 +1543,11 @@ build_dir_tree(struct inode *dp, uint64 pinum) // Path lookup // =========================================================================== +// Copy the next component of `path` into `name` (VFS_NAME_MAX + 1 bytes) and +// return what follows it, or 0 at the end. A component longer than +// VFS_NAME_MAX sets *toolong and ends the walk. static char * -skipelem(char *path, char *name) +skipelem(char *path, char *name, int *toolong) { char *s; int len; @@ -1551,12 +1560,12 @@ skipelem(char *path, char *name) while (*path != '/' && *path != 0) path++; len = path - s; - if (len >= DIRSIZ) - memmove(name, s, DIRSIZ); - else { - memmove(name, s, len); - name[len] = 0; + if (len > VFS_NAME_MAX) { + *toolong = 1; + return 0; } + memmove(name, s, len); + name[len] = 0; while (*path == '/') path++; return path; @@ -1764,14 +1773,27 @@ namex_from(struct inode *start, char *path, int nameiparent, char *name, int follow, int *err) { struct inode *ip, *next; - int links = 0; + int links = 0, toolong = 0; - // A private copy, because following a link rewrites what is left of it. - char safe[MAXPATH]; - safestrcpy(safe, path, MAXPATH); - char *spath = safe; if (err) *err = ENOENT; + if (strnlen(path, VFS_PATH_MAX) >= VFS_PATH_MAX) { + if (err) + *err = ENAMETOOLONG; + return 0; + } + + // A private copy, because following a link rewrites what is left of it. + // A page each rather than on the stack: PATH_MAX twice over is the whole of + // a kernel stack's slack, and a page is exactly PATH_MAX. + char *safe = kalloc(), *tgt = 0; + if (!safe) { + if (err) + *err = ENOMEM; + return 0; + } + safestrcpy(safe, path, VFS_PATH_MAX); + char *spath = safe; // Root is a SchFS object, not a v1 inode. ops_for(SCHFS_DEV) then routes // every operation on it through schfs_ops, which is the whole point: the v1 @@ -1780,7 +1802,7 @@ namex_from(struct inode *start, char *path, int nameiparent, char *name, // The root itself can be a mount point, and a relative walk can start on one. ip = mount_cross(ip); - while ((spath = skipelem(spath, name)) != 0) { + while ((spath = skipelem(spath, name, &toolong)) != 0) { // ".." out of a mounted filesystem's root leaves it. The filesystem down // there does not know it is mounted anywhere and would answer with its own // root, which turns the mount into a subtree you can enter and never @@ -1800,15 +1822,15 @@ namex_from(struct inode *start, char *path, int nameiparent, char *name, iunlockput(ip); if (err) *err = ENOTDIR; - return 0; + goto fail; } if (nameiparent && *spath == '\0') { iunlock(ip); - return ip; + goto out; } if ((next = dirlookup(ip, name, 0)) == 0) { iunlockput(ip); - return 0; + goto fail; } iunlock(ip); next = mount_cross(next); @@ -1823,16 +1845,22 @@ namex_from(struct inode *start, char *path, int nameiparent, char *name, // A link: put its target in front of what is left of the path and carry // on from the directory it was found in, or from root. - char tgt[MAXPATH]; - int len = (++links > MAXSYMLINKS) ? -1 - : readi(next, 0, (uint64)tgt, 0, MAXPATH - 1); + if (!tgt && !(tgt = kalloc())) { + iunlockput(next); + iput(ip); + if (err) + *err = ENOMEM; + goto fail; + } + int len = (++links > MAXSYMLINKS) ? -1 + : readi(next, 0, (uint64)tgt, 0, VFS_PATH_MAX - 1); iunlockput(next); int rest = strlen(spath); - if (len <= 0 || len + 1 + rest >= MAXPATH) { + if (len <= 0 || len + 1 + rest >= VFS_PATH_MAX) { iput(ip); if (err) *err = len < 0 ? ELOOP : len == 0 ? ENOENT : ENAMETOOLONG; - return 0; + goto fail; } tgt[len] = '\0'; if (rest) { @@ -1847,11 +1875,26 @@ namex_from(struct inode *start, char *path, int nameiparent, char *name, } } + if (toolong) { + iput(ip); + if (err) + *err = ENAMETOOLONG; + goto fail; + } if (nameiparent) { iput(ip); - return 0; + goto fail; } +out: + kfree(safe); + if (tgt) + kfree(tgt); return ip; +fail: + kfree(safe); + if (tgt) + kfree(tgt); + return 0; } static struct inode * @@ -1863,7 +1906,7 @@ namex(char *path, int nameiparent, char *name) struct inode * namei(char *path) { - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; return namex(path, 0, name); } @@ -1878,7 +1921,7 @@ nameiparent(char *path, char *name) struct inode * nameifrom(struct inode *start, char *path) { - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; return namex_from(start, path, 0, name, 0, 0); } @@ -1894,7 +1937,7 @@ nameiparentfrom(struct inode *start, char *path, char *name) struct inode * nameifollow(struct inode *start, char *path, int *err) { - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; return namex_from(start ? start : myproc()->cwd, path, 0, name, 1, err); } diff --git a/kernel/sys/sys_fs.c b/kernel/sys/sys_fs.c index eb5c04d..92c5d8d 100644 --- a/kernel/sys/sys_fs.c +++ b/kernel/sys/sys_fs.c @@ -186,14 +186,31 @@ at_nameiparent(int dirfd, char *path, char *name) return nameiparentfrom(start, path, name); } +// 1 if some component of `path` is longer than NAME_MAX, which is why a +// lookup through nameiparent() failed when it did. It reports no reason. +static int +path_toolong(const char *path) +{ + int n = 0; + if (strnlen(path, VFS_PATH_MAX) >= VFS_PATH_MAX) return 1; + for (; *path; path++) { + n = (*path == '/') ? 0 : n + 1; + if (n > VFS_NAME_MAX) return 1; + } + return 0; +} + static struct inode* createat_x(int dirfd, char *path, short type, short major, short minor, int *err) { struct inode *ip, *dp; - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; uint off; - if (!(dp = at_nameiparent(dirfd, path, name))) return 0; + if (!(dp = at_nameiparent(dirfd, path, name))) { + if (err && path && path_toolong(path)) *err = ENAMETOOLONG; + return 0; + } ilock(dp); // `err` lets the caller report why. Returning 0 alone made a refused // write say "No such file or directory", which is what a shell prints for @@ -884,7 +901,7 @@ addr_t sys_mknod(void) // renameat() is corrected alongside rename() because musl's rename() calls it. addr_t sys_link(void) { - char name[DIRSIZ], *new, *old; + char name[VFS_NAME_MAX + 1], *new, *old; struct inode *dp, *ip; int err = EPERM; if (argstr(0, &old) < 0 || argstr(1, &new) < 0) return -EFAULT; @@ -936,7 +953,7 @@ int unlink_path(char *path) { struct inode *ip, *dp; - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; uint off; int err = EPERM; @@ -1067,7 +1084,7 @@ addr_t sys_rename(void) { char *oldpath, *newpath; struct inode *ip, *dp_old, *dp_new, *ip_src, *ip_dst; - char oldname[DIRSIZ], newname[DIRSIZ]; + char oldname[VFS_NAME_MAX + 1], newname[VFS_NAME_MAX + 1]; uint off, dstoff; int err = EPERM; if (argstr(0, &oldpath) < 0 || argstr(1, &newpath) < 0) return -EFAULT; @@ -1244,7 +1261,7 @@ sys_getdents64(void) addr_t sys_delete(void) { struct inode *ip, *dp, *ip2; - char name[DIRSIZ], *path; + char name[VFS_NAME_MAX + 1], *path; uint off; if (argstr(0, &path) < 0) return -1; begin_op(); @@ -1897,7 +1914,7 @@ addr_t sys_unlinkat(void) int dirfd, flags; char *path; struct inode *ip, *dp; - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; uint off; argint(0, &dirfd); if (argstr(1, &path) < 0) return -1; @@ -1937,7 +1954,7 @@ addr_t sys_renameat(void) // Delegate to sys_rename internals — fake the args for rename. // Easiest: place path pointers in registers and call rename logic directly. struct inode *ip, *dp_old, *dp_new, *ip_src; - char oldname[DIRSIZ], newname[DIRSIZ]; + char oldname[VFS_NAME_MAX + 1], newname[VFS_NAME_MAX + 1]; uint off; struct dirent de; begin_op(); @@ -1981,7 +1998,7 @@ addr_t sys_linkat(void) int olddirfd, newdirfd; char *old, *new; struct inode *dp, *ip; - char name[DIRSIZ]; + char name[VFS_NAME_MAX + 1]; argint(0, &olddirfd); argstr(1, &old); argint(2, &newdirfd); argstr(3, &new); argint(4, &(int){0}); // flags