summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorJamie Liu <jamieliu@google.com>2019-04-25 16:03:32 -0700
committerShentubot <shentubot@google.com>2019-04-25 16:05:13 -0700
commit6b76c172b48ecb2c342882c0fe6474b2b973dad0 (patch)
tree61295be888beb8cb9c024aad03b9debe44fc2e32
parent72197810405a462386ded05d3c7c60c320289223 (diff)
Don't enforce NAME_MAX in fs.Dirent.walk().
Maximum filename length is filesystem-dependent, and obtained via statfs::f_namelen. This limit is usually 255 bytes (NAME_MAX), but not always. For example, VFAT supports filenames of up to 255... UCS-2 characters, which Linux conservatively takes to mean UTF-8-encoded bytes: fs/fat/inode.c:fat_statfs(), FAT_LFN_LEN * NLS_MAX_CHARSET_SIZE. As a result, Linux's VFS does not enforce NAME_MAX: $ rg --maxdepth=1 '\WNAME_MAX\W' fs/ include/linux/ fs/libfs.c 38: buf->f_namelen = NAME_MAX; 64: if (dentry->d_name.len > NAME_MAX) include/linux/relay.h 74: char base_filename[NAME_MAX]; /* saved base filename */ include/linux/fscrypt.h 149: * filenames up to NAME_MAX bytes, since base64 encoding expands the length. include/linux/exportfs.h 176: * understanding that it is already pointing to a a %NAME_MAX+1 sized Remove this check from core VFS, and add it to ramfs (and by extension tmpfs), where it is actually applicable: mm/shmem.c:shmem_dir_inode_operations.lookup == simple_lookup *does* enforce NAME_MAX. PiperOrigin-RevId: 245324748 Change-Id: I17567c4324bfd60e31746a5270096e75db963fac
-rw-r--r--pkg/sentry/fs/dirent.go5
-rw-r--r--pkg/sentry/fs/gofer/path.go40
-rw-r--r--pkg/sentry/fs/ramfs/dir.go23
-rw-r--r--test/syscalls/linux/chdir.cc5
-rw-r--r--test/syscalls/linux/statfs.cc1
5 files changed, 64 insertions, 10 deletions
diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go
index 4870e7d40..4bcdf530a 100644
--- a/pkg/sentry/fs/dirent.go
+++ b/pkg/sentry/fs/dirent.go
@@ -459,11 +459,6 @@ func (d *Dirent) walk(ctx context.Context, root *Dirent, name string, walkMayUnl
return nil, syscall.ENOTDIR
}
- // The component must be less than NAME_MAX.
- if len(name) > linux.NAME_MAX {
- return nil, syscall.ENAMETOOLONG
- }
-
if name == "" || name == "." {
d.IncRef()
return d, nil
diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go
index 5e1a8b623..8ae33d286 100644
--- a/pkg/sentry/fs/gofer/path.go
+++ b/pkg/sentry/fs/gofer/path.go
@@ -27,9 +27,17 @@ import (
"gvisor.googlesource.com/gvisor/pkg/syserror"
)
+// maxFilenameLen is the maximum length of a filename. This is dictated by 9P's
+// encoding of strings, which uses 2 bytes for the length prefix.
+const maxFilenameLen = (1 << 16) - 1
+
// Lookup loads an Inode at name into a Dirent based on the session's cache
// policy.
func (i *inodeOperations) Lookup(ctx context.Context, dir *fs.Inode, name string) (*fs.Dirent, error) {
+ if len(name) > maxFilenameLen {
+ return nil, syserror.ENAMETOOLONG
+ }
+
cp := i.session().cachePolicy
if cp.cacheReaddir() {
// Check to see if we have readdirCache that indicates the
@@ -72,6 +80,10 @@ func (i *inodeOperations) Lookup(ctx context.Context, dir *fs.Inode, name string
//
// Ownership is currently ignored.
func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string, flags fs.FileFlags, perm fs.FilePermissions) (*fs.File, error) {
+ if len(name) > maxFilenameLen {
+ return nil, syserror.ENAMETOOLONG
+ }
+
// Create replaces the directory fid with the newly created/opened
// file, so clone this directory so it doesn't change out from under
// this node.
@@ -139,6 +151,10 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string
// CreateLink uses Create to create a symlink between oldname and newname.
func (i *inodeOperations) CreateLink(ctx context.Context, dir *fs.Inode, oldname string, newname string) error {
+ if len(newname) > maxFilenameLen {
+ return syserror.ENAMETOOLONG
+ }
+
owner := fs.FileOwnerFromContext(ctx)
if _, err := i.fileState.file.symlink(ctx, oldname, newname, p9.UID(owner.UID), p9.GID(owner.GID)); err != nil {
return err
@@ -149,6 +165,10 @@ func (i *inodeOperations) CreateLink(ctx context.Context, dir *fs.Inode, oldname
// CreateHardLink implements InodeOperations.CreateHardLink.
func (i *inodeOperations) CreateHardLink(ctx context.Context, inode *fs.Inode, target *fs.Inode, newName string) error {
+ if len(newName) > maxFilenameLen {
+ return syserror.ENAMETOOLONG
+ }
+
targetOpts, ok := target.InodeOperations.(*inodeOperations)
if !ok {
return syscall.EXDEV
@@ -167,6 +187,10 @@ func (i *inodeOperations) CreateHardLink(ctx context.Context, inode *fs.Inode, t
// CreateDirectory uses Create to create a directory named s under inodeOperations.
func (i *inodeOperations) CreateDirectory(ctx context.Context, dir *fs.Inode, s string, perm fs.FilePermissions) error {
+ if len(s) > maxFilenameLen {
+ return syserror.ENAMETOOLONG
+ }
+
owner := fs.FileOwnerFromContext(ctx)
if _, err := i.fileState.file.mkdir(ctx, s, p9.FileMode(perm.LinuxMode()), p9.UID(owner.UID), p9.GID(owner.GID)); err != nil {
return err
@@ -184,6 +208,10 @@ func (i *inodeOperations) CreateDirectory(ctx context.Context, dir *fs.Inode, s
// Bind implements InodeOperations.Bind.
func (i *inodeOperations) Bind(ctx context.Context, dir *fs.Inode, name string, ep transport.BoundEndpoint, perm fs.FilePermissions) (*fs.Dirent, error) {
+ if len(name) > maxFilenameLen {
+ return nil, syserror.ENAMETOOLONG
+ }
+
if i.session().endpoints == nil {
return nil, syscall.EOPNOTSUPP
}
@@ -252,6 +280,10 @@ func (*inodeOperations) CreateFifo(context.Context, *fs.Inode, string, fs.FilePe
// Remove implements InodeOperations.Remove.
func (i *inodeOperations) Remove(ctx context.Context, dir *fs.Inode, name string) error {
+ if len(name) > maxFilenameLen {
+ return syserror.ENAMETOOLONG
+ }
+
var key device.MultiDeviceKey
removeSocket := false
if i.session().endpoints != nil {
@@ -284,6 +316,10 @@ func (i *inodeOperations) Remove(ctx context.Context, dir *fs.Inode, name string
// Remove implements InodeOperations.RemoveDirectory.
func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, name string) error {
+ if len(name) > maxFilenameLen {
+ return syserror.ENAMETOOLONG
+ }
+
// 0x200 = AT_REMOVEDIR.
if err := i.fileState.file.unlinkAt(ctx, name, 0x200); err != nil {
return err
@@ -301,6 +337,10 @@ func (i *inodeOperations) RemoveDirectory(ctx context.Context, dir *fs.Inode, na
// Rename renames this node.
func (i *inodeOperations) Rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent *fs.Inode, newName string, replacement bool) error {
+ if len(newName) > maxFilenameLen {
+ return syserror.ENAMETOOLONG
+ }
+
// Unwrap the new parent to a *inodeOperations.
newParentInodeOperations, ok := newParent.InodeOperations.(*inodeOperations)
if !ok {
diff --git a/pkg/sentry/fs/ramfs/dir.go b/pkg/sentry/fs/ramfs/dir.go
index 011cf3a16..159fd2981 100644
--- a/pkg/sentry/fs/ramfs/dir.go
+++ b/pkg/sentry/fs/ramfs/dir.go
@@ -192,6 +192,10 @@ func (d *Dir) removeChildLocked(ctx context.Context, name string) (*fs.Inode, er
// Remove removes the named non-directory.
func (d *Dir) Remove(ctx context.Context, _ *fs.Inode, name string) error {
+ if len(name) > linux.NAME_MAX {
+ return syserror.ENAMETOOLONG
+ }
+
d.mu.Lock()
defer d.mu.Unlock()
inode, err := d.removeChildLocked(ctx, name)
@@ -206,6 +210,10 @@ func (d *Dir) Remove(ctx context.Context, _ *fs.Inode, name string) error {
// RemoveDirectory removes the named directory.
func (d *Dir) RemoveDirectory(ctx context.Context, _ *fs.Inode, name string) error {
+ if len(name) > linux.NAME_MAX {
+ return syserror.ENAMETOOLONG
+ }
+
d.mu.Lock()
defer d.mu.Unlock()
@@ -234,6 +242,10 @@ func (d *Dir) RemoveDirectory(ctx context.Context, _ *fs.Inode, name string) err
// Lookup loads an inode at p into a Dirent.
func (d *Dir) Lookup(ctx context.Context, _ *fs.Inode, p string) (*fs.Dirent, error) {
+ if len(p) > linux.NAME_MAX {
+ return nil, syserror.ENAMETOOLONG
+ }
+
d.mu.Lock()
defer d.mu.Unlock()
@@ -265,6 +277,10 @@ func (d *Dir) walkLocked(ctx context.Context, p string) (*fs.Inode, error) {
// createInodeOperationsCommon creates a new child node at this dir by calling
// makeInodeOperations. It is the common logic for creating a new child.
func (d *Dir) createInodeOperationsCommon(ctx context.Context, name string, makeInodeOperations func() (*fs.Inode, error)) (*fs.Inode, error) {
+ if len(name) > linux.NAME_MAX {
+ return nil, syserror.ENAMETOOLONG
+ }
+
d.mu.Lock()
defer d.mu.Unlock()
@@ -314,6 +330,10 @@ func (d *Dir) CreateLink(ctx context.Context, dir *fs.Inode, oldname, newname st
// CreateHardLink creates a new hard link.
func (d *Dir) CreateHardLink(ctx context.Context, dir *fs.Inode, target *fs.Inode, name string) error {
+ if len(name) > linux.NAME_MAX {
+ return syserror.ENAMETOOLONG
+ }
+
d.mu.Lock()
defer d.mu.Unlock()
@@ -465,6 +485,9 @@ func Rename(ctx context.Context, oldParent fs.InodeOperations, oldName string, n
if !ok {
return syserror.EXDEV
}
+ if len(newName) > linux.NAME_MAX {
+ return syserror.ENAMETOOLONG
+ }
np.mu.Lock()
defer np.mu.Unlock()
diff --git a/test/syscalls/linux/chdir.cc b/test/syscalls/linux/chdir.cc
index 4905ffb23..a4b54f0ee 100644
--- a/test/syscalls/linux/chdir.cc
+++ b/test/syscalls/linux/chdir.cc
@@ -54,11 +54,6 @@ TEST(ChdirTest, NotDir) {
EXPECT_THAT(chdir(temp_file.path().c_str()), SyscallFailsWithErrno(ENOTDIR));
}
-TEST(ChdirTest, NameTooLong) {
- std::string name(NAME_MAX + 1, 'a');
- ASSERT_THAT(chdir(name.c_str()), SyscallFailsWithErrno(ENAMETOOLONG));
-}
-
TEST(ChdirTest, NotExist) {
EXPECT_THAT(chdir("/foo/bar"), SyscallFailsWithErrno(ENOENT));
}
diff --git a/test/syscalls/linux/statfs.cc b/test/syscalls/linux/statfs.cc
index 1fc9758c9..e1e7fc707 100644
--- a/test/syscalls/linux/statfs.cc
+++ b/test/syscalls/linux/statfs.cc
@@ -49,6 +49,7 @@ TEST(StatfsTest, NameLen) {
struct statfs st;
EXPECT_THAT(statfs("/dev/shm", &st), SyscallSucceeds());
+ // This assumes that /dev/shm is tmpfs.
EXPECT_EQ(st.f_namelen, NAME_MAX);
}