diff options
Diffstat (limited to 'pkg/sentry/fs')
42 files changed, 95 insertions, 95 deletions
diff --git a/pkg/sentry/fs/README.md b/pkg/sentry/fs/README.md index a88a0cd3a..f53ed3eaa 100644 --- a/pkg/sentry/fs/README.md +++ b/pkg/sentry/fs/README.md @@ -59,7 +59,7 @@ two categories: The first is always necessary to save and restore. An application may never have any open file descriptors, but across save and restore it should see a coherent -view of any mount namespace. NOTE: Currently only one "initial" +view of any mount namespace. NOTE(b/63601033): Currently only one "initial" mount namespace is supported. The second is so that system calls across save and restore are coherent with diff --git a/pkg/sentry/fs/ashmem/area.go b/pkg/sentry/fs/ashmem/area.go index 651cbc164..1f61c5711 100644 --- a/pkg/sentry/fs/ashmem/area.go +++ b/pkg/sentry/fs/ashmem/area.go @@ -240,7 +240,7 @@ func (a *Area) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArgume return 0, syserror.EINVAL } - // TODO: If personality flag + // TODO(b/30946773,gvisor.dev/issue/153): If personality flag // READ_IMPLIES_EXEC is set, set PROT_EXEC if PORT_READ is set. a.perms = perms @@ -290,7 +290,7 @@ func (a *Area) pinOperation(pin linux.AshmemPin, op uint32) (uintptr, error) { return linux.AshmemNotPurged, nil case linux.AshmemUnpinIoctl: - // TODO: Implement purge on unpin. + // TODO(b/30946773): Implement purge on unpin. a.pb.UnpinRange(r) return 0, nil diff --git a/pkg/sentry/fs/binder/binder.go b/pkg/sentry/fs/binder/binder.go index a41b5dcae..d9f1559de 100644 --- a/pkg/sentry/fs/binder/binder.go +++ b/pkg/sentry/fs/binder/binder.go @@ -69,7 +69,7 @@ func NewDevice(ctx context.Context, owner fs.FileOwner, fp fs.FilePermissions) * // GetFile implements fs.InodeOperations.GetFile. // -// TODO: Add functionality to GetFile: Additional fields will be +// TODO(b/30946773): Add functionality to GetFile: Additional fields will be // needed in the Device structure, initialize them here. Also, Device will need // to keep track of the created Procs in order to implement BINDER_READ_WRITE // ioctl. @@ -133,7 +133,7 @@ func (bp *Proc) Write(ctx context.Context, file *fs.File, src usermem.IOSequence // Flush implements fs.FileOperations.Flush. // -// TODO: Implement. +// TODO(b/30946773): Implement. func (bp *Proc) Flush(ctx context.Context, file *fs.File) error { return nil } @@ -149,7 +149,7 @@ func (bp *Proc) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.M } opts.MaxPerms.Write = false - // TODO: Binder sets VM_DONTCOPY, preventing the created vma + // TODO(b/30946773): Binder sets VM_DONTCOPY, preventing the created vma // from being copied across fork(), but we don't support this yet. As // a result, MMs containing a Binder mapping cannot be forked (MM.Fork will // fail when AddMapping returns EBUSY). @@ -159,7 +159,7 @@ func (bp *Proc) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.M // Ioctl implements fs.FileOperations.Ioctl. // -// TODO: Implement. +// TODO(b/30946773): Implement. func (bp *Proc) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // Switch on ioctl request. switch uint32(args[1].Int()) { @@ -173,22 +173,22 @@ func (bp *Proc) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArgum }) return 0, err case linux.BinderWriteReadIoctl: - // TODO: Implement. + // TODO(b/30946773): Implement. fallthrough case linux.BinderSetIdleTimeoutIoctl: - // TODO: Implement. + // TODO(b/30946773): Implement. fallthrough case linux.BinderSetMaxThreadsIoctl: - // TODO: Implement. + // TODO(b/30946773): Implement. fallthrough case linux.BinderSetIdlePriorityIoctl: - // TODO: Implement. + // TODO(b/30946773): Implement. fallthrough case linux.BinderSetContextMgrIoctl: - // TODO: Implement. + // TODO(b/30946773): Implement. fallthrough case linux.BinderThreadExitIoctl: - // TODO: Implement. + // TODO(b/30946773): Implement. return 0, syserror.ENOSYS default: // Ioctls irrelevant to Binder. @@ -228,7 +228,7 @@ func (bp *Proc) CopyMapping(ctx context.Context, ms memmap.MappingSpace, srcAR, // Translate implements memmap.Mappable.Translate. func (bp *Proc) Translate(ctx context.Context, required, optional memmap.MappableRange, at usermem.AccessType) ([]memmap.Translation, error) { - // TODO: In addition to the page initially allocated and mapped + // TODO(b/30946773): In addition to the page initially allocated and mapped // in AddMapping (Linux: binder_mmap), Binder allocates and maps pages for // each transaction (Linux: binder_ioctl => binder_ioctl_write_read => // binder_thread_write => binder_transaction => binder_alloc_buf => diff --git a/pkg/sentry/fs/dentry.go b/pkg/sentry/fs/dentry.go index 4879df4d6..29fb155a4 100644 --- a/pkg/sentry/fs/dentry.go +++ b/pkg/sentry/fs/dentry.go @@ -83,7 +83,7 @@ type DirCtx struct { attrs map[string]DentAttr // DirCursor is the directory cursor. - // TODO: Once Handles are removed this can just live in the + // TODO(b/67778717): Once Handles are removed this can just live in the // respective FileOperations implementations and not need to get // plumbed everywhere. DirCursor *string diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go index 4bcdf530a..54fc11fe1 100644 --- a/pkg/sentry/fs/dirent.go +++ b/pkg/sentry/fs/dirent.go @@ -318,7 +318,7 @@ func (d *Dirent) SyncAll(ctx context.Context) { // There is nothing to sync for a read-only filesystem. if !d.Inode.MountSource.Flags.ReadOnly { - // FIXME: This should be a mount traversal, not a + // FIXME(b/34856369): This should be a mount traversal, not a // Dirent traversal, because some Inodes that need to be synced // may no longer be reachable by name (after sys_unlink). // @@ -1506,7 +1506,7 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string } // Are we frozen? - // TODO: Is this the right errno? + // TODO(jamieliu): Is this the right errno? if oldParent.frozen && !oldParent.Inode.IsVirtual() { return syscall.ENOENT } @@ -1565,7 +1565,7 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string } else { // Check constraints on the dirent being replaced. - // NOTE: We don't want to keep replaced alive + // NOTE(b/111808347): We don't want to keep replaced alive // across the Rename, so must call DecRef manually (no defer). // Check that we can delete replaced. @@ -1606,7 +1606,7 @@ func Rename(ctx context.Context, root *Dirent, oldParent *Dirent, oldName string // Allow the file system to drop extra references on replaced. replaced.dropExtendedReference() - // NOTE: Keeping a dirent + // NOTE(b/31798319,b/31867149,b/31867671): Keeping a dirent // open across renames is currently broken for multiple // reasons, so we flush all references on the replaced node and // its children. diff --git a/pkg/sentry/fs/file.go b/pkg/sentry/fs/file.go index 2c2126f17..5d5026661 100644 --- a/pkg/sentry/fs/file.go +++ b/pkg/sentry/fs/file.go @@ -65,7 +65,7 @@ const FileMaxOffset = math.MaxInt64 // under a single abortable mutex which also synchronizes lseek(2), read(2), // and write(2). // -// FIXME: Split synchronization from cancellation. +// FIXME(b/38451980): Split synchronization from cancellation. // // +stateify savable type File struct { diff --git a/pkg/sentry/fs/file_overlay.go b/pkg/sentry/fs/file_overlay.go index e1f02f0f4..6e680f0a4 100644 --- a/pkg/sentry/fs/file_overlay.go +++ b/pkg/sentry/fs/file_overlay.go @@ -160,7 +160,7 @@ func (f *overlayFileOperations) Seek(ctx context.Context, file *File, whence See // If this was a seek on a directory, we must update the cursor. if seekDir && whence == SeekSet && offset == 0 { // Currently only seeking to 0 on a directory is supported. - // FIXME: Lift directory seeking limitations. + // FIXME(b/33075855): Lift directory seeking limitations. f.dirCursor = "" } return n, nil @@ -329,7 +329,7 @@ func (*overlayFileOperations) ConfigureMMap(ctx context.Context, file *File, opt if !o.isMappableLocked() { return syserror.ENODEV } - // FIXME: This is a copy/paste of fsutil.GenericConfigureMMap, + // FIXME(jamieliu): This is a copy/paste of fsutil.GenericConfigureMMap, // which we can't use because the overlay implementation is in package fs, // so depending on fs/fsutil would create a circular dependency. Move // overlay to fs/overlay. diff --git a/pkg/sentry/fs/fsutil/file.go b/pkg/sentry/fs/fsutil/file.go index df34dc788..42afdd11c 100644 --- a/pkg/sentry/fs/fsutil/file.go +++ b/pkg/sentry/fs/fsutil/file.go @@ -36,7 +36,7 @@ func (FileNoopRelease) Release() {} // // Currently only seeking to 0 on a directory is supported. // -// FIXME: Lift directory seeking limitations. +// FIXME(b/33075855): Lift directory seeking limitations. func SeekWithDirCursor(ctx context.Context, file *fs.File, whence fs.SeekWhence, offset int64, dirCursor *string) (int64, error) { inode := file.Dirent.Inode current := file.Offset() @@ -50,7 +50,7 @@ func SeekWithDirCursor(ctx context.Context, file *fs.File, whence fs.SeekWhence, if fs.IsCharDevice(inode.StableAttr) { // Ignore seek requests. // - // FIXME: This preserves existing + // FIXME(b/34716638): This preserves existing // behavior but is not universally correct. return 0, nil } @@ -104,7 +104,7 @@ func SeekWithDirCursor(ctx context.Context, file *fs.File, whence fs.SeekWhence, return current, syserror.EINVAL } return sz + offset, nil - // FIXME: This is not universally correct. + // FIXME(b/34778850): This is not universally correct. // Remove SpecialDirectory. case fs.SpecialDirectory: if offset != 0 { @@ -112,7 +112,7 @@ func SeekWithDirCursor(ctx context.Context, file *fs.File, whence fs.SeekWhence, } // SEEK_END to 0 moves the directory "cursor" to the end. // - // FIXME: The ensures that after the seek, + // FIXME(b/35442290): The ensures that after the seek, // reading on the directory will get EOF. But it is not // correct in general because the directory can grow in // size; attempting to read those new entries will be diff --git a/pkg/sentry/fs/fsutil/inode_cached.go b/pkg/sentry/fs/fsutil/inode_cached.go index b690cfe93..ba33b9912 100644 --- a/pkg/sentry/fs/fsutil/inode_cached.go +++ b/pkg/sentry/fs/fsutil/inode_cached.go @@ -479,7 +479,7 @@ func (c *CachingInodeOperations) Read(ctx context.Context, file *fs.File, dst us // common: getting a return value of 0 from a read syscall is the only way // to detect EOF. // - // TODO: Separate out c.attr.Size and use atomics instead of + // TODO(jamieliu): Separate out c.attr.Size and use atomics instead of // c.dataMu. c.dataMu.RLock() size := c.attr.Size @@ -776,7 +776,7 @@ func (c *CachingInodeOperations) Translate(ctx context.Context, required, option var translatedEnd uint64 for seg := c.cache.FindSegment(required.Start); seg.Ok() && seg.Start() < required.End; seg, _ = seg.NextNonEmpty() { segMR := seg.Range().Intersect(optional) - // TODO: Make Translations writable even if writability is + // TODO(jamieliu): Make Translations writable even if writability is // not required if already kept-dirty by another writable translation. perms := usermem.AccessType{ Read: true, diff --git a/pkg/sentry/fs/gofer/cache_policy.go b/pkg/sentry/fs/gofer/cache_policy.go index d7fbb71b7..51c573aef 100644 --- a/pkg/sentry/fs/gofer/cache_policy.go +++ b/pkg/sentry/fs/gofer/cache_policy.go @@ -136,7 +136,7 @@ func (cp cachePolicy) revalidate(ctx context.Context, name string, parent, child // Walk from parent to child again. // - // TODO: If we have a directory FD in the parent + // TODO(b/112031682): If we have a directory FD in the parent // inodeOperations, then we can use fstatat(2) to get the inode // attributes instead of making this RPC. qids, _, mask, attr, err := parentIops.fileState.file.walkGetAttr(ctx, []string{name}) @@ -171,7 +171,7 @@ func (cp cachePolicy) keep(d *fs.Dirent) bool { return false } sattr := d.Inode.StableAttr - // NOTE: Only cache files, directories, and symlinks. + // NOTE(b/31979197): Only cache files, directories, and symlinks. return fs.IsFile(sattr) || fs.IsDir(sattr) || fs.IsSymlink(sattr) } diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go index 80d1e08a6..35caa42cd 100644 --- a/pkg/sentry/fs/gofer/file.go +++ b/pkg/sentry/fs/gofer/file.go @@ -297,7 +297,7 @@ func (f *fileOperations) Flush(ctx context.Context, file *fs.File) error { // We do this because some p9 server implementations of Flush are // over-zealous. // - // FIXME: weaken these implementations and remove this check. + // FIXME(edahlgren): weaken these implementations and remove this check. if !file.Flags().Write { return nil } diff --git a/pkg/sentry/fs/gofer/file_state.go b/pkg/sentry/fs/gofer/file_state.go index f770ca4ea..d0c64003c 100644 --- a/pkg/sentry/fs/gofer/file_state.go +++ b/pkg/sentry/fs/gofer/file_state.go @@ -28,7 +28,7 @@ func (f *fileOperations) afterLoad() { // Manually load the open handles. var err error - // TODO: Context is not plumbed to save/restore. + // TODO(b/38173783): Context is not plumbed to save/restore. f.handles, err = f.inodeOperations.fileState.getHandles(context.Background(), f.flags) if err != nil { return fmt.Errorf("failed to re-open handle: %v", err) diff --git a/pkg/sentry/fs/gofer/handles.go b/pkg/sentry/fs/gofer/handles.go index f32e99ce0..0b33e80c3 100644 --- a/pkg/sentry/fs/gofer/handles.go +++ b/pkg/sentry/fs/gofer/handles.go @@ -49,7 +49,7 @@ func (h *handles) DecRef() { log.Warningf("error closing host file: %v", err) } } - // FIXME: Context is not plumbed here. + // FIXME(b/38173783): Context is not plumbed here. if err := h.File.close(context.Background()); err != nil { log.Warningf("error closing p9 file: %v", err) } diff --git a/pkg/sentry/fs/gofer/inode.go b/pkg/sentry/fs/gofer/inode.go index 29af1010c..1181a24cc 100644 --- a/pkg/sentry/fs/gofer/inode.go +++ b/pkg/sentry/fs/gofer/inode.go @@ -570,13 +570,13 @@ func init() { } // AddLink implements InodeOperations.AddLink, but is currently a noop. -// FIXME: Remove this from InodeOperations altogether. +// FIXME(b/63117438): Remove this from InodeOperations altogether. func (*inodeOperations) AddLink() {} // DropLink implements InodeOperations.DropLink, but is currently a noop. -// FIXME: Remove this from InodeOperations altogether. +// FIXME(b/63117438): Remove this from InodeOperations altogether. func (*inodeOperations) DropLink() {} // NotifyStatusChange implements fs.InodeOperations.NotifyStatusChange. -// FIXME: Remove this from InodeOperations altogether. +// FIXME(b/63117438): Remove this from InodeOperations altogether. func (i *inodeOperations) NotifyStatusChange(ctx context.Context) {} diff --git a/pkg/sentry/fs/gofer/inode_state.go b/pkg/sentry/fs/gofer/inode_state.go index ad4d3df58..44d76ba9f 100644 --- a/pkg/sentry/fs/gofer/inode_state.go +++ b/pkg/sentry/fs/gofer/inode_state.go @@ -123,7 +123,7 @@ func (i *inodeFileState) afterLoad() { // beforeSave. return fmt.Errorf("failed to find path for inode number %d. Device %s contains %s", i.sattr.InodeID, i.s.connID, fs.InodeMappings(i.s.inodeMappings)) } - // TODO: Context is not plumbed to save/restore. + // TODO(b/38173783): Context is not plumbed to save/restore. ctx := &dummyClockContext{context.Background()} _, i.file, err = i.s.attach.walk(ctx, splitAbsolutePath(name)) diff --git a/pkg/sentry/fs/gofer/session.go b/pkg/sentry/fs/gofer/session.go index ed5147c65..4ed688ce5 100644 --- a/pkg/sentry/fs/gofer/session.go +++ b/pkg/sentry/fs/gofer/session.go @@ -134,7 +134,7 @@ type session struct { // socket files. This allows unix domain sockets to be used with paths that // belong to a gofer. // - // TODO: there are few possible races with someone stat'ing the + // TODO(b/77154739): there are few possible races with someone stat'ing the // file and another deleting it concurrently, where the file will not be // reported as socket file. endpoints *endpointMaps `state:"wait"` diff --git a/pkg/sentry/fs/gofer/session_state.go b/pkg/sentry/fs/gofer/session_state.go index 0ad5d63b5..b1f299be5 100644 --- a/pkg/sentry/fs/gofer/session_state.go +++ b/pkg/sentry/fs/gofer/session_state.go @@ -104,7 +104,7 @@ func (s *session) afterLoad() { // If private unix sockets are enabled, create and fill the session's endpoint // maps. if opts.privateunixsocket { - // TODO: Context is not plumbed to save/restore. + // TODO(b/38173783): Context is not plumbed to save/restore. ctx := &dummyClockContext{context.Background()} if err = s.restoreEndpointMaps(ctx); err != nil { diff --git a/pkg/sentry/fs/host/fs.go b/pkg/sentry/fs/host/fs.go index 800649211..de349a41a 100644 --- a/pkg/sentry/fs/host/fs.go +++ b/pkg/sentry/fs/host/fs.go @@ -87,7 +87,7 @@ func (f *Filesystem) Mount(ctx context.Context, _ string, flags fs.MountSourceFl options := fs.GenericMountSourceOptions(data) // Grab the whitelist if one was specified. - // TODO: require another option "testonly" in order to allow + // TODO(edahlgren/mpratt/hzy): require another option "testonly" in order to allow // no whitelist. if wl, ok := options[whitelistKey]; ok { f.paths = strings.Split(wl, "|") @@ -320,7 +320,7 @@ func (m *superOperations) SaveInodeMapping(inode *fs.Inode, path string) { // Keep implements fs.MountSourceOperations.Keep. // -// TODO: It is possible to change the permissions on a +// TODO(b/72455313,b/77596690): It is possible to change the permissions on a // host file while it is in the dirent cache (say from RO to RW), but it is not // possible to re-open the file with more relaxed permissions, since the host // FD is already open and stored in the inode. diff --git a/pkg/sentry/fs/host/inode.go b/pkg/sentry/fs/host/inode.go index 2030edcb4..69c648f67 100644 --- a/pkg/sentry/fs/host/inode.go +++ b/pkg/sentry/fs/host/inode.go @@ -95,7 +95,7 @@ type inodeFileState struct { // ReadToBlocksAt implements fsutil.CachedFileObject.ReadToBlocksAt. func (i *inodeFileState) ReadToBlocksAt(ctx context.Context, dsts safemem.BlockSeq, offset uint64) (uint64, error) { - // TODO: Using safemem.FromIOReader here is wasteful for two + // TODO(jamieliu): Using safemem.FromIOReader here is wasteful for two // reasons: // // - Using preadv instead of iterated preads saves on host system calls. @@ -325,7 +325,7 @@ func (i *inodeOperations) GetFile(ctx context.Context, d *fs.Dirent, flags fs.Fi // canMap returns true if this fs.Inode can be memory mapped. func canMap(inode *fs.Inode) bool { - // FIXME: Some obscure character devices can be mapped. + // FIXME(b/38213152): Some obscure character devices can be mapped. return fs.IsFile(inode.StableAttr) } @@ -428,15 +428,15 @@ func (i *inodeOperations) StatFS(context.Context) (fs.Info, error) { } // AddLink implements fs.InodeOperations.AddLink. -// FIXME: Remove this from InodeOperations altogether. +// FIXME(b/63117438): Remove this from InodeOperations altogether. func (i *inodeOperations) AddLink() {} // DropLink implements fs.InodeOperations.DropLink. -// FIXME: Remove this from InodeOperations altogether. +// FIXME(b/63117438): Remove this from InodeOperations altogether. func (i *inodeOperations) DropLink() {} // NotifyStatusChange implements fs.InodeOperations.NotifyStatusChange. -// FIXME: Remove this from InodeOperations altogether. +// FIXME(b/63117438): Remove this from InodeOperations altogether. func (i *inodeOperations) NotifyStatusChange(ctx context.Context) {} // readdirAll returns all of the directory entries in i. diff --git a/pkg/sentry/fs/inode.go b/pkg/sentry/fs/inode.go index d82f9740e..fe411a766 100644 --- a/pkg/sentry/fs/inode.go +++ b/pkg/sentry/fs/inode.go @@ -93,10 +93,10 @@ func (i *Inode) DecRef() { // destroy releases the Inode and releases the msrc reference taken. func (i *Inode) destroy() { - // FIXME: Context is not plumbed here. + // FIXME(b/38173783): Context is not plumbed here. ctx := context.Background() if err := i.WriteOut(ctx); err != nil { - // FIXME: Mark as warning again once noatime is + // FIXME(b/65209558): Mark as warning again once noatime is // properly supported. log.Debugf("Inode %+v, failed to sync all metadata: %v", i.StableAttr, err) } @@ -359,7 +359,7 @@ func (i *Inode) Getlink(ctx context.Context) (*Dirent, error) { // AddLink calls i.InodeOperations.AddLink. func (i *Inode) AddLink() { if i.overlay != nil { - // FIXME: Remove this from InodeOperations altogether. + // FIXME(b/63117438): Remove this from InodeOperations altogether. // // This interface is only used by ramfs to update metadata of // children. These filesystems should _never_ have overlay diff --git a/pkg/sentry/fs/inode_operations.go b/pkg/sentry/fs/inode_operations.go index ceacc7659..ff8b75f31 100644 --- a/pkg/sentry/fs/inode_operations.go +++ b/pkg/sentry/fs/inode_operations.go @@ -118,7 +118,7 @@ type InodeOperations interface { // // The caller must ensure that this operation is permitted. // - // TODO: merge Remove and RemoveDirectory, Remove + // TODO(b/67778723): merge Remove and RemoveDirectory, Remove // just needs a type flag. Remove(ctx context.Context, dir *Inode, name string) error diff --git a/pkg/sentry/fs/inode_overlay.go b/pkg/sentry/fs/inode_overlay.go index 254646176..bda3e1861 100644 --- a/pkg/sentry/fs/inode_overlay.go +++ b/pkg/sentry/fs/inode_overlay.go @@ -142,7 +142,7 @@ func overlayLookup(ctx context.Context, parent *overlayEntry, inode *Inode, name } else { // If we have something from the upper, we can only use it if the types // match. - // NOTE: Allow SpecialDirectories and Directories to merge. + // NOTE(b/112312863): Allow SpecialDirectories and Directories to merge. // This is needed to allow submounts in /proc and /sys. if upperInode.StableAttr.Type == child.Inode.StableAttr.Type || (IsDir(upperInode.StableAttr) && IsDir(child.Inode.StableAttr)) { @@ -226,7 +226,7 @@ func overlayCreate(ctx context.Context, o *overlayEntry, parent *Dirent, name st return nil, err } - // NOTE: Replace the Dirent with a transient Dirent, since + // NOTE(b/71766861): Replace the Dirent with a transient Dirent, since // we are about to create the real Dirent: an overlay Dirent. // // This ensures the *fs.File returned from overlayCreate is in the same @@ -338,7 +338,7 @@ func overlayRename(ctx context.Context, o *overlayEntry, oldParent *Dirent, rena // directory will appear empty in the upper fs, which will then // allow the rename to proceed when it should return ENOTEMPTY. // - // NOTE: Ideally, we'd just pass in the replaced + // NOTE(b/111808347): Ideally, we'd just pass in the replaced // Dirent from Rename, but we must drop the reference on // replaced before we make the rename call, so Rename can't // pass the Dirent to the Inode without significantly diff --git a/pkg/sentry/fs/mount.go b/pkg/sentry/fs/mount.go index 1e245ae5f..4d1693204 100644 --- a/pkg/sentry/fs/mount.go +++ b/pkg/sentry/fs/mount.go @@ -42,7 +42,7 @@ type DirentOperations interface { // MountSourceOperations contains filesystem specific operations. type MountSourceOperations interface { - // TODO: Add: + // TODO(b/67778729): Add: // BlockSize() int64 // FS() Filesystem @@ -101,7 +101,7 @@ func (i InodeMappings) String() string { // amalgamation implies that a mount source cannot be shared by multiple mounts // (e.g. cannot be mounted at different locations). // -// TODO: Move mount-specific information out of MountSource. +// TODO(b/63601033): Move mount-specific information out of MountSource. // // +stateify savable type MountSource struct { diff --git a/pkg/sentry/fs/mount_test.go b/pkg/sentry/fs/mount_test.go index 269d6b9da..d7605b2c9 100644 --- a/pkg/sentry/fs/mount_test.go +++ b/pkg/sentry/fs/mount_test.go @@ -33,7 +33,7 @@ func cacheReallyContains(cache *DirentCache, d *Dirent) bool { } // TestMountSourceOnlyCachedOnce tests that a Dirent that is mounted over only ends -// up in a single Dirent Cache. NOTE: Having a dirent in multiple +// up in a single Dirent Cache. NOTE(b/63848693): Having a dirent in multiple // caches causes major consistency issues. func TestMountSourceOnlyCachedOnce(t *testing.T) { ctx := contexttest.Context(t) diff --git a/pkg/sentry/fs/proc/README.md b/pkg/sentry/fs/proc/README.md index 3cc5f197c..5d4ec6c7b 100644 --- a/pkg/sentry/fs/proc/README.md +++ b/pkg/sentry/fs/proc/README.md @@ -91,7 +91,7 @@ CPU.IO utilization in last 10 minutes | Always zero Num currently running processes | Always zero Total num processes | Always zero -TODO: Populate the columns with accurate statistics. +TODO(b/62345059): Populate the columns with accurate statistics. ### meminfo @@ -128,12 +128,12 @@ Field name | Notes Buffers | Always zero, no block devices SwapCache | Always zero, no swap Inactive(anon) | Always zero, see SwapCache -Unevictable | Always zero TODO -Mlocked | Always zero TODO +Unevictable | Always zero TODO(b/31823263) +Mlocked | Always zero TODO(b/31823263) SwapTotal | Always zero, no swap SwapFree | Always zero, no swap -Dirty | Always zero TODO -Writeback | Always zero TODO +Dirty | Always zero TODO(b/31823263) +Writeback | Always zero TODO(b/31823263) MemAvailable | Uses the same value as MemFree since there is no swap. Slab | Missing SReclaimable | Missing @@ -185,7 +185,7 @@ softirq 0 0 0 0 0 0 0 0 0 0 0 All fields except for `btime` are always zero. -TODO: Populate with accurate fields. +TODO(b/37226836): Populate with accurate fields. ### sys diff --git a/pkg/sentry/fs/proc/fds.go b/pkg/sentry/fs/proc/fds.go index 25da06f5d..f2329e623 100644 --- a/pkg/sentry/fs/proc/fds.go +++ b/pkg/sentry/fs/proc/fds.go @@ -258,7 +258,7 @@ func newFdInfoDir(t *kernel.Task, msrc *fs.MountSource) *fs.Inode { // Lookup loads an fd in /proc/TID/fdinfo into a Dirent. func (fdid *fdInfoDir) Lookup(ctx context.Context, dir *fs.Inode, p string) (*fs.Dirent, error) { inode, err := walkDescriptors(fdid.t, p, func(file *fs.File, fdFlags kernel.FDFlags) *fs.Inode { - // TODO: Using a static inode here means that the + // TODO(b/121266871): Using a static inode here means that the // data can be out-of-date if, for instance, the flags on the // FD change before we read this file. We should switch to // generating the data on Read(). Also, we should include pos, diff --git a/pkg/sentry/fs/proc/loadavg.go b/pkg/sentry/fs/proc/loadavg.go index 78f3a1dc0..3ee0e570a 100644 --- a/pkg/sentry/fs/proc/loadavg.go +++ b/pkg/sentry/fs/proc/loadavg.go @@ -40,7 +40,7 @@ func (d *loadavgData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) var buf bytes.Buffer - // TODO: Include real data in fields. + // TODO(b/62345059): Include real data in fields. // Column 1-3: CPU and IO utilization of the last 1, 5, and 10 minute periods. // Column 4-5: currently running processes and the total number of processes. // Column 6: the last process ID used. diff --git a/pkg/sentry/fs/proc/meminfo.go b/pkg/sentry/fs/proc/meminfo.go index 620e93ce3..75cbf3e77 100644 --- a/pkg/sentry/fs/proc/meminfo.go +++ b/pkg/sentry/fs/proc/meminfo.go @@ -58,7 +58,7 @@ func (d *meminfoData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) fmt.Fprintf(&buf, "MemTotal: %8d kB\n", totalSize/1024) memFree := (totalSize - totalUsage) / 1024 // We use MemFree as MemAvailable because we don't swap. - // TODO: When reclaim is implemented the value of MemAvailable + // TODO(rahat): When reclaim is implemented the value of MemAvailable // should change. fmt.Fprintf(&buf, "MemFree: %8d kB\n", memFree) fmt.Fprintf(&buf, "MemAvailable: %8d kB\n", memFree) @@ -72,8 +72,8 @@ func (d *meminfoData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) fmt.Fprintf(&buf, "Inactive(anon): 0 kB\n") fmt.Fprintf(&buf, "Active(file): %8d kB\n", activeFile/1024) fmt.Fprintf(&buf, "Inactive(file): %8d kB\n", inactiveFile/1024) - fmt.Fprintf(&buf, "Unevictable: 0 kB\n") // TODO - fmt.Fprintf(&buf, "Mlocked: 0 kB\n") // TODO + fmt.Fprintf(&buf, "Unevictable: 0 kB\n") // TODO(b/31823263) + fmt.Fprintf(&buf, "Mlocked: 0 kB\n") // TODO(b/31823263) fmt.Fprintf(&buf, "SwapTotal: 0 kB\n") fmt.Fprintf(&buf, "SwapFree: 0 kB\n") fmt.Fprintf(&buf, "Dirty: 0 kB\n") diff --git a/pkg/sentry/fs/proc/mounts.go b/pkg/sentry/fs/proc/mounts.go index 1e62af8c6..fe62b167b 100644 --- a/pkg/sentry/fs/proc/mounts.go +++ b/pkg/sentry/fs/proc/mounts.go @@ -114,7 +114,7 @@ func (mif *mountInfoFile) ReadSeqFileData(ctx context.Context, handle seqfile.Se // (4) Root: the pathname of the directory in the filesystem // which forms the root of this mount. // - // NOTE: This will always be "/" until we implement + // NOTE(b/78135857): This will always be "/" until we implement // bind mounts. fmt.Fprintf(&buf, "/ ") diff --git a/pkg/sentry/fs/proc/net.go b/pkg/sentry/fs/proc/net.go index 55a958f9e..d24b2d370 100644 --- a/pkg/sentry/fs/proc/net.go +++ b/pkg/sentry/fs/proc/net.go @@ -154,7 +154,7 @@ func (n *netDev) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) ([]se contents[1] = " face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed\n" for _, i := range interfaces { - // TODO: Collect stats from each inet.Stack + // TODO(b/71872867): Collect stats from each inet.Stack // implementation (hostinet, epsocket, and rpcinet). // Implements the same format as diff --git a/pkg/sentry/fs/proc/stat.go b/pkg/sentry/fs/proc/stat.go index f2bbef375..18bd8e9b6 100644 --- a/pkg/sentry/fs/proc/stat.go +++ b/pkg/sentry/fs/proc/stat.go @@ -83,7 +83,7 @@ func (s *statData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) ([] var buf bytes.Buffer - // TODO: We currently export only zero CPU stats. We could + // TODO(b/37226836): We currently export only zero CPU stats. We could // at least provide some aggregate stats. var cpu cpuStats fmt.Fprintf(&buf, "cpu %s\n", cpu) @@ -100,7 +100,7 @@ func (s *statData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) ([] const numInterrupts = 256 // The Kernel doesn't handle real interrupts, so report all zeroes. - // TODO: We could count page faults as #PF. + // TODO(b/37226836): We could count page faults as #PF. fmt.Fprintf(&buf, "intr 0") // total for i := 0; i < numInterrupts; i++ { fmt.Fprintf(&buf, " 0") @@ -108,22 +108,22 @@ func (s *statData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) ([] fmt.Fprintf(&buf, "\n") // Total number of context switches. - // TODO: Count this. + // TODO(b/37226836): Count this. fmt.Fprintf(&buf, "ctxt 0\n") // CLOCK_REALTIME timestamp from boot, in seconds. fmt.Fprintf(&buf, "btime %d\n", s.k.Timekeeper().BootTime().Seconds()) // Total number of clones. - // TODO: Count this. + // TODO(b/37226836): Count this. fmt.Fprintf(&buf, "processes 0\n") // Number of runnable tasks. - // TODO: Count this. + // TODO(b/37226836): Count this. fmt.Fprintf(&buf, "procs_running 0\n") // Number of tasks waiting on IO. - // TODO: Count this. + // TODO(b/37226836): Count this. fmt.Fprintf(&buf, "procs_blocked 0\n") // Number of each softirq handled. diff --git a/pkg/sentry/fs/proc/sys_net.go b/pkg/sentry/fs/proc/sys_net.go index 728a46a74..0ce77f04f 100644 --- a/pkg/sentry/fs/proc/sys_net.go +++ b/pkg/sentry/fs/proc/sys_net.go @@ -39,7 +39,7 @@ const ( // tcpMemInode is used to read/write the size of netstack tcp buffers. // -// TODO: If we have multiple proc mounts, concurrent writes can +// TODO(b/121381035): If we have multiple proc mounts, concurrent writes can // leave netstack and the proc files in an inconsistent state. Since we set the // buffer size from these proc files on restore, we may also race and end up in // an inconsistent state on restore. diff --git a/pkg/sentry/fs/proc/task.go b/pkg/sentry/fs/proc/task.go index 0edcdfce2..9f65a8337 100644 --- a/pkg/sentry/fs/proc/task.go +++ b/pkg/sentry/fs/proc/task.go @@ -77,7 +77,7 @@ func newTaskDir(t *kernel.Task, msrc *fs.MountSource, pidns *kernel.PIDNamespace "fd": newFdDir(t, msrc), "fdinfo": newFdInfoDir(t, msrc), "gid_map": newGIDMap(t, msrc), - // FIXME: create the correct io file for threads. + // FIXME(b/123511468): create the correct io file for threads. "io": newIO(t, msrc), "maps": newMaps(t, msrc), "mountinfo": seqfile.NewSeqFileInode(t, &mountInfoFile{t: t}, msrc), @@ -93,7 +93,7 @@ func newTaskDir(t *kernel.Task, msrc *fs.MountSource, pidns *kernel.PIDNamespace contents["task"] = newSubtasks(t, msrc, pidns) } - // TODO: Set EUID/EGID based on dumpability. + // TODO(b/31916171): Set EUID/EGID based on dumpability. d := &taskDir{ Dir: *ramfs.NewDir(t, contents, fs.RootOwner, fs.FilePermsFromMode(0555)), t: t, @@ -245,7 +245,7 @@ func (e *exe) executable() (d *fs.Dirent, err error) { e.t.WithMuLocked(func(t *kernel.Task) { mm := t.MemoryManager() if mm == nil { - // TODO: Check shouldn't allow Readlink once the + // TODO(b/34851096): Check shouldn't allow Readlink once the // Task is zombied. err = syserror.EACCES return @@ -297,7 +297,7 @@ type namespaceSymlink struct { } func newNamespaceSymlink(t *kernel.Task, msrc *fs.MountSource, name string) *fs.Inode { - // TODO: Namespace symlinks should contain the namespace name and the + // TODO(rahat): Namespace symlinks should contain the namespace name and the // inode number for the namespace instance, so for example user:[123456]. We // currently fake the inode number by sticking the symlink inode in its // place. diff --git a/pkg/sentry/fs/proc/version.go b/pkg/sentry/fs/proc/version.go index b6d49d5e9..58e0c793c 100644 --- a/pkg/sentry/fs/proc/version.go +++ b/pkg/sentry/fs/proc/version.go @@ -65,7 +65,7 @@ func (v *versionData) ReadSeqFileData(ctx context.Context, h seqfile.SeqHandle) // Since we don't really want to expose build information to // applications, those fields are omitted. // - // FIXME: Using Version from the init task SyscallTable + // FIXME(mpratt): Using Version from the init task SyscallTable // disregards the different version a task may have (e.g., in a uts // namespace). ver := init.Leader().SyscallTable().Version diff --git a/pkg/sentry/fs/ramfs/dir.go b/pkg/sentry/fs/ramfs/dir.go index 159fd2981..c0400b67d 100644 --- a/pkg/sentry/fs/ramfs/dir.go +++ b/pkg/sentry/fs/ramfs/dir.go @@ -358,7 +358,7 @@ func (d *Dir) CreateDirectory(ctx context.Context, dir *fs.Inode, name string, p _, err := d.createInodeOperationsCommon(ctx, name, func() (*fs.Inode, error) { return d.NewDir(ctx, dir, perms) }) - // TODO: Support updating status times, as those should be + // TODO(nlacasse): Support updating status times, as those should be // updated by links. return err } diff --git a/pkg/sentry/fs/tmpfs/fs.go b/pkg/sentry/fs/tmpfs/fs.go index d0c93028f..8e44421b6 100644 --- a/pkg/sentry/fs/tmpfs/fs.go +++ b/pkg/sentry/fs/tmpfs/fs.go @@ -34,7 +34,7 @@ const ( // GID for the root directory. rootGIDKey = "gid" - // TODO: support a tmpfs size limit. + // TODO(edahlgren/mpratt): support a tmpfs size limit. // size = "size" // Permissions that exceed modeMask will be rejected. diff --git a/pkg/sentry/fs/tmpfs/inode_file.go b/pkg/sentry/fs/tmpfs/inode_file.go index 7c80d711b..4450e1363 100644 --- a/pkg/sentry/fs/tmpfs/inode_file.go +++ b/pkg/sentry/fs/tmpfs/inode_file.go @@ -309,7 +309,7 @@ func (f *fileInodeOperations) read(ctx context.Context, file *fs.File, dst userm // common: getting a return value of 0 from a read syscall is the only way // to detect EOF. // - // TODO: Separate out f.attr.Size and use atomics instead of + // TODO(jamieliu): Separate out f.attr.Size and use atomics instead of // f.dataMu. f.dataMu.RLock() size := f.attr.Size diff --git a/pkg/sentry/fs/tmpfs/tmpfs.go b/pkg/sentry/fs/tmpfs/tmpfs.go index 555692505..5bb4922cb 100644 --- a/pkg/sentry/fs/tmpfs/tmpfs.go +++ b/pkg/sentry/fs/tmpfs/tmpfs.go @@ -32,7 +32,7 @@ import ( var fsInfo = fs.Info{ Type: linux.TMPFS_MAGIC, - // TODO: allow configuring a tmpfs size and enforce it. + // TODO(b/29637826): allow configuring a tmpfs size and enforce it. TotalBlocks: 0, FreeBlocks: 0, } diff --git a/pkg/sentry/fs/tty/dir.go b/pkg/sentry/fs/tty/dir.go index 33b4c6438..f8713471a 100644 --- a/pkg/sentry/fs/tty/dir.go +++ b/pkg/sentry/fs/tty/dir.go @@ -66,7 +66,7 @@ type dirInodeOperations struct { // msrc is the super block this directory is on. // - // TODO: Plumb this through instead of storing it here. + // TODO(chrisko): Plumb this through instead of storing it here. msrc *fs.MountSource // mu protects the fields below. @@ -89,7 +89,7 @@ type dirInodeOperations struct { // next is the next pty index to use. // - // TODO: reuse indices when ptys are closed. + // TODO(b/29356795): reuse indices when ptys are closed. next uint32 } @@ -118,7 +118,7 @@ func newDir(ctx context.Context, m *fs.MountSource) *fs.Inode { // N.B. Linux always uses inode id 1 for the directory. See // fs/devpts/inode.c:devpts_fill_super. // - // TODO: Since ptsDevice must be shared between + // TODO(b/75267214): Since ptsDevice must be shared between // different mounts, we must not assign fixed numbers. InodeID: ptsDevice.NextIno(), BlockSize: usermem.PageSize, diff --git a/pkg/sentry/fs/tty/fs.go b/pkg/sentry/fs/tty/fs.go index 43e0e2a04..a53448c47 100644 --- a/pkg/sentry/fs/tty/fs.go +++ b/pkg/sentry/fs/tty/fs.go @@ -43,7 +43,7 @@ func (*filesystem) Name() string { // AllowUserMount allows users to mount(2) this file system. func (*filesystem) AllowUserMount() bool { - // TODO: Users may mount this once the terminals are in a + // TODO(b/29356795): Users may mount this once the terminals are in a // usable state. return false } diff --git a/pkg/sentry/fs/tty/master.go b/pkg/sentry/fs/tty/master.go index 7c256abb0..e2686a074 100644 --- a/pkg/sentry/fs/tty/master.go +++ b/pkg/sentry/fs/tty/master.go @@ -51,7 +51,7 @@ func newMasterInode(ctx context.Context, d *dirInodeOperations, owner fs.FileOwn // N.B. Linux always uses inode id 2 for ptmx. See // fs/devpts/inode.c:mknod_ptmx. // - // TODO: Since ptsDevice must be shared between + // TODO(b/75267214): Since ptsDevice must be shared between // different mounts, we must not assign fixed numbers. InodeID: ptsDevice.NextIno(), Type: fs.CharacterDevice, @@ -157,7 +157,7 @@ func (mf *masterFileOperations) Ioctl(ctx context.Context, io usermem.IO, args a // of the slave end. return mf.t.ld.setTermios(ctx, io, args) case linux.TCSETSW: - // TODO: This should drain the output queue first. + // TODO(b/29356795): This should drain the output queue first. return mf.t.ld.setTermios(ctx, io, args) case linux.TIOCGPTN: _, err := usermem.CopyObjectOut(ctx, io, args[2].Pointer(), uint32(mf.t.n), usermem.IOOpts{ @@ -165,7 +165,7 @@ func (mf *masterFileOperations) Ioctl(ctx context.Context, io usermem.IO, args a }) return 0, err case linux.TIOCSPTLCK: - // TODO: Implement pty locking. For now just pretend we do. + // TODO(b/29356795): Implement pty locking. For now just pretend we do. return 0, nil case linux.TIOCGWINSZ: return 0, mf.t.ld.windowSize(ctx, io, args) diff --git a/pkg/sentry/fs/tty/slave.go b/pkg/sentry/fs/tty/slave.go index e8368bcdd..ed080ca0f 100644 --- a/pkg/sentry/fs/tty/slave.go +++ b/pkg/sentry/fs/tty/slave.go @@ -56,7 +56,7 @@ func newSlaveInode(ctx context.Context, d *dirInodeOperations, t *Terminal, owne // N.B. Linux always uses inode id = tty index + 3. See // fs/devpts/inode.c:devpts_pty_new. // - // TODO: Since ptsDevice must be shared between + // TODO(b/75267214): Since ptsDevice must be shared between // different mounts, we must not assign fixed numbers. InodeID: ptsDevice.NextIno(), Type: fs.CharacterDevice, @@ -137,7 +137,7 @@ func (sf *slaveFileOperations) Ioctl(ctx context.Context, io usermem.IO, args ar case linux.TCSETS: return sf.si.t.ld.setTermios(ctx, io, args) case linux.TCSETSW: - // TODO: This should drain the output queue first. + // TODO(b/29356795): This should drain the output queue first. return sf.si.t.ld.setTermios(ctx, io, args) case linux.TIOCGPTN: _, err := usermem.CopyObjectOut(ctx, io, args[2].Pointer(), uint32(sf.si.t.n), usermem.IOOpts{ @@ -151,7 +151,7 @@ func (sf *slaveFileOperations) Ioctl(ctx context.Context, io usermem.IO, args ar case linux.TIOCSCTTY: // Make the given terminal the controlling terminal of the // calling process. - // TODO: Implement once we have support for job + // TODO(b/129283598): Implement once we have support for job // control. return 0, nil default: |