diff options
| author | Nicolas Lacasse <nlacasse@google.com> | 2019-04-29 14:03:04 -0700 |
|---|---|---|
| committer | Shentubot <shentubot@google.com> | 2019-04-29 14:04:14 -0700 |
| commit | f4ce43e1f426148d99c28c1b0e5c43ddda17a8cb (patch) | |
| tree | ef64d18350874742742599c8b059b333eb060920 /pkg | |
| parent | 38e627644756400413fffe7222cdd5200dc4eccf (diff) | |
Allow and document bug ids in gVisor codebase.
PiperOrigin-RevId: 245818639
Change-Id: I03703ef0fb9b6675955637b9fe2776204c545789
Diffstat (limited to 'pkg')
121 files changed, 263 insertions, 263 deletions
diff --git a/pkg/cpuid/cpuid_test.go b/pkg/cpuid/cpuid_test.go index 35e7b8e50..64ade1cbe 100644 --- a/pkg/cpuid/cpuid_test.go +++ b/pkg/cpuid/cpuid_test.go @@ -78,7 +78,7 @@ func TestTakeFeatureIntersection(t *testing.T) { } } -// TODO: Run this test on a very old platform, and make sure more +// TODO(b/73346484): Run this test on a very old platform, and make sure more // bits are enabled than just FPU and PAE. This test currently may not detect // if HostFeatureSet gives back junk bits. func TestHostFeatureSet(t *testing.T) { diff --git a/pkg/dhcp/client.go b/pkg/dhcp/client.go index 354205e63..2ba79be32 100644 --- a/pkg/dhcp/client.go +++ b/pkg/dhcp/client.go @@ -120,7 +120,7 @@ func (c *Client) Config() Config { // If the server sets a lease limit a timer is set to automatically // renew it. func (c *Client) Request(ctx context.Context, requestedAddr tcpip.Address) (cfg Config, reterr error) { - // TODO: remove calls to {Add,Remove}Address when they're no + // TODO(b/127321246): remove calls to {Add,Remove}Address when they're no // longer required to send and receive broadcast. if err := c.stack.AddAddressWithOptions(c.nicid, ipv4.ProtocolNumber, tcpipHeader.IPv4Any, stack.NeverPrimaryEndpoint); err != nil && err != tcpip.ErrDuplicateAddress { return Config{}, fmt.Errorf("dhcp: AddAddressWithOptions(): %s", err) diff --git a/pkg/log/glog.go b/pkg/log/glog.go index fbb58501b..24d5390d7 100644 --- a/pkg/log/glog.go +++ b/pkg/log/glog.go @@ -144,7 +144,7 @@ func (g GoogleEmitter) Emit(level Level, timestamp time.Time, format string, arg b.writeAll(pid) b.write(' ') - // FIXME: The caller, fabricated. This really sucks, but it + // FIXME(b/73383460): The caller, fabricated. This really sucks, but it // is unacceptable to put runtime.Callers() in the hot path. b.writeAll(caller) b.write(']') diff --git a/pkg/metric/metric.go b/pkg/metric/metric.go index 02af75974..e5eb95f89 100644 --- a/pkg/metric/metric.go +++ b/pkg/metric/metric.go @@ -44,8 +44,8 @@ var ( // // Metrics are not saved across save/restore and thus reset to zero on restore. // -// TODO: Support non-cumulative metrics. -// TODO: Support metric fields. +// TODO(b/67298402): Support non-cumulative metrics. +// TODO(b/67298427): Support metric fields. // type Uint64Metric struct { // value is the actual value of the metric. It must be accessed diff --git a/pkg/segment/set.go b/pkg/segment/set.go index a9a3b8875..74a916ea3 100644 --- a/pkg/segment/set.go +++ b/pkg/segment/set.go @@ -1270,7 +1270,7 @@ func segmentAfterPosition(n *node, i int) Iterator { } func zeroValueSlice(slice []Value) { - // TODO: check if Go is actually smart enough to optimize a + // TODO(jamieliu): check if Go is actually smart enough to optimize a // ClearValue that assigns nil to a memset here for i := range slice { Functions{}.ClearValue(&slice[i]) diff --git a/pkg/segment/test/set_functions.go b/pkg/segment/test/set_functions.go index 05ba5fbb9..41f649011 100644 --- a/pkg/segment/test/set_functions.go +++ b/pkg/segment/test/set_functions.go @@ -15,7 +15,7 @@ package segment // Basic numeric constants that we define because the math package doesn't. -// TODO: These should be Math.MaxInt64/MinInt64? +// TODO(nlacasse): These should be Math.MaxInt64/MinInt64? const ( maxInt = int(^uint(0) >> 1) minInt = -maxInt - 1 diff --git a/pkg/sentry/arch/arch.go b/pkg/sentry/arch/arch.go index 4cd7a9af5..16d8eb2b2 100644 --- a/pkg/sentry/arch/arch.go +++ b/pkg/sentry/arch/arch.go @@ -53,7 +53,7 @@ type FloatingPointData byte // Context provides architecture-dependent information for a specific thread. // -// NOTE: Currently we use uintptr here to refer to a generic native +// NOTE(b/34169503): Currently we use uintptr here to refer to a generic native // register value. While this will work for the foreseeable future, it isn't // strictly correct. We may want to create some abstraction that makes this // more clear or enables us to store values of arbitrary widths. This is diff --git a/pkg/sentry/arch/arch_amd64.go b/pkg/sentry/arch/arch_amd64.go index 2507774f7..7ec2f2c84 100644 --- a/pkg/sentry/arch/arch_amd64.go +++ b/pkg/sentry/arch/arch_amd64.go @@ -305,7 +305,7 @@ func (c *context64) PtracePeekUser(addr uintptr) (interface{}, error) { buf := binary.Marshal(nil, usermem.ByteOrder, c.ptraceGetRegs()) return c.Native(uintptr(usermem.ByteOrder.Uint64(buf[addr:]))), nil } - // TODO: debug registers + // TODO(b/34088053): debug registers return c.Native(0), nil } @@ -320,6 +320,6 @@ func (c *context64) PtracePokeUser(addr, data uintptr) error { _, err := c.PtraceSetRegs(bytes.NewBuffer(buf)) return err } - // TODO: debug registers + // TODO(b/34088053): debug registers return nil } diff --git a/pkg/sentry/arch/arch_x86.go b/pkg/sentry/arch/arch_x86.go index c8bf0e7f2..4305fe2cb 100644 --- a/pkg/sentry/arch/arch_x86.go +++ b/pkg/sentry/arch/arch_x86.go @@ -306,7 +306,7 @@ func (s *State) ptraceGetRegs() syscall.PtraceRegs { // FS/GS_TLS_SEL when fs_base/gs_base is a 64-bit value. (We do the // same in PtraceSetRegs.) // - // TODO: Remove this fixup since newer Linux + // TODO(gvisor.dev/issue/168): Remove this fixup since newer Linux // doesn't have this behavior anymore. if regs.Fs == 0 && regs.Fs_base <= 0xffffffff { regs.Fs = _FS_TLS_SEL diff --git a/pkg/sentry/arch/signal_amd64.go b/pkg/sentry/arch/signal_amd64.go index c9de36897..7f76eba27 100644 --- a/pkg/sentry/arch/signal_amd64.go +++ b/pkg/sentry/arch/signal_amd64.go @@ -319,7 +319,7 @@ func (c *context64) NewSignalStack() NativeSignalStack { // From Linux 'arch/x86/include/uapi/asm/sigcontext.h' the following is the // size of the magic cookie at the end of the xsave frame. // -// NOTE: Currently we don't actually populate the fpstate +// NOTE(b/33003106#comment11): Currently we don't actually populate the fpstate // on the signal stack. const _FP_XSTATE_MAGIC2_SIZE = 4 @@ -392,7 +392,7 @@ func (c *context64) SignalSetup(st *Stack, act *SignalAct, info *SignalInfo, alt Sigset: sigset, } - // TODO: Set SignalContext64.Err, Trapno, and Cr2 + // TODO(gvisor.dev/issue/159): Set SignalContext64.Err, Trapno, and Cr2 // based on the fault that caused the signal. For now, leave Err and // Trapno unset and assume CR2 == info.Addr() for SIGSEGVs and // SIGBUSes. @@ -505,7 +505,7 @@ func (c *context64) SignalRestore(st *Stack, rt bool) (linux.SignalSet, SignalSt l := len(c.sigFPState) if l > 0 { c.x86FPState = c.sigFPState[l-1] - // NOTE: State save requires that any slice + // NOTE(cl/133042258): State save requires that any slice // elements from '[len:cap]' to be zero value. c.sigFPState[l-1] = nil c.sigFPState = c.sigFPState[0 : l-1] diff --git a/pkg/sentry/arch/stack.go b/pkg/sentry/arch/stack.go index f2cfb0426..2e33ccdf5 100644 --- a/pkg/sentry/arch/stack.go +++ b/pkg/sentry/arch/stack.go @@ -97,7 +97,7 @@ func (s *Stack) Push(vals ...interface{}) (usermem.Addr, error) { if c < 0 { return 0, fmt.Errorf("bad binary.Size for %T", v) } - // TODO: Use a real context.Context. + // TODO(b/38173783): Use a real context.Context. n, err := usermem.CopyObjectOut(context.Background(), s.IO, s.Bottom-usermem.Addr(c), norm, usermem.IOOpts{}) if err != nil || c != n { return 0, err @@ -121,11 +121,11 @@ func (s *Stack) Pop(vals ...interface{}) (usermem.Addr, error) { var err error if isVaddr { value := s.Arch.Native(uintptr(0)) - // TODO: Use a real context.Context. + // TODO(b/38173783): Use a real context.Context. n, err = usermem.CopyObjectIn(context.Background(), s.IO, s.Bottom, value, usermem.IOOpts{}) *vaddr = usermem.Addr(s.Arch.Value(value)) } else { - // TODO: Use a real context.Context. + // TODO(b/38173783): Use a real context.Context. n, err = usermem.CopyObjectIn(context.Background(), s.IO, s.Bottom, v, usermem.IOOpts{}) } if err != nil { diff --git a/pkg/sentry/context/context.go b/pkg/sentry/context/context.go index 7ed6a5e8a..eefc3e1b4 100644 --- a/pkg/sentry/context/context.go +++ b/pkg/sentry/context/context.go @@ -114,7 +114,7 @@ var bgContext = &logContext{Logger: log.Log()} // Background returns an empty context using the default logger. // // Users should be wary of using a Background context. Please tag any use with -// FIXME and a note to remove this use. +// FIXME(b/38173783) and a note to remove this use. // // Generally, one should use the Task as their context when available, or avoid // having to use a context in places where a Task is unavailable. diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index e848def14..aca2267a7 100644 --- a/pkg/sentry/control/proc.go +++ b/pkg/sentry/control/proc.go @@ -261,7 +261,7 @@ func (proc *Proc) Ps(args *PsArgs, out *string) error { } // Process contains information about a single process in a Sandbox. -// TODO: Implement TTY field. +// TODO(b/117881927): Implement TTY field. type Process struct { UID auth.KUID `json:"uid"` PID kernel.ThreadID `json:"pid"` 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: diff --git a/pkg/sentry/kernel/auth/credentials.go b/pkg/sentry/kernel/auth/credentials.go index a843b9aab..2055da196 100644 --- a/pkg/sentry/kernel/auth/credentials.go +++ b/pkg/sentry/kernel/auth/credentials.go @@ -125,7 +125,7 @@ func NewUserCredentials(kuid KUID, kgid KGID, extraKGIDs []KGID, capabilities *T creds.EffectiveCaps = capabilities.EffectiveCaps creds.BoundingCaps = capabilities.BoundingCaps creds.InheritableCaps = capabilities.InheritableCaps - // TODO: Support ambient capabilities. + // TODO(nlacasse): Support ambient capabilities. } else { // If no capabilities are specified, grant capabilities consistent with // setresuid + setresgid from NewRootCredentials to the given uid and diff --git a/pkg/sentry/kernel/auth/user_namespace.go b/pkg/sentry/kernel/auth/user_namespace.go index 30957bb9a..159940a69 100644 --- a/pkg/sentry/kernel/auth/user_namespace.go +++ b/pkg/sentry/kernel/auth/user_namespace.go @@ -49,7 +49,7 @@ type UserNamespace struct { gidMapFromParent idMapSet gidMapToParent idMapSet - // TODO: Support disabling setgroups(2). + // TODO(b/27454212): Support disabling setgroups(2). } // NewRootUserNamespace returns a UserNamespace that is appropriate for a diff --git a/pkg/sentry/kernel/pending_signals.go b/pkg/sentry/kernel/pending_signals.go index 373e11772..deff6def9 100644 --- a/pkg/sentry/kernel/pending_signals.go +++ b/pkg/sentry/kernel/pending_signals.go @@ -30,7 +30,7 @@ const ( // rtSignalCap is the maximum number of instances of a given realtime // signal that may be pending. // - // TODO: In Linux, the minimum signal queue size is + // TODO(igudger): In Linux, the minimum signal queue size is // RLIMIT_SIGPENDING, which is by default max_threads/2. rtSignalCap = 32 ) diff --git a/pkg/sentry/kernel/ptrace.go b/pkg/sentry/kernel/ptrace.go index 8d78b2fb3..15f2e2964 100644 --- a/pkg/sentry/kernel/ptrace.go +++ b/pkg/sentry/kernel/ptrace.go @@ -162,7 +162,7 @@ func (t *Task) CanTrace(target *Task, attach bool) bool { if cgid := callerCreds.RealKGID; cgid != targetCreds.RealKGID || cgid != targetCreds.EffectiveKGID || cgid != targetCreds.SavedKGID { return false } - // TODO: dumpability check + // TODO(b/31916171): dumpability check if callerCreds.UserNamespace != targetCreds.UserNamespace { return false } @@ -396,7 +396,7 @@ func (t *Task) ptraceAttach(target *Task, seize bool, opts uintptr) error { if target.stop == (*groupStop)(nil) { target.trapStopPending = true target.endInternalStopLocked() - // TODO: Linux blocks ptrace_attach() until the task has + // TODO(jamieliu): Linux blocks ptrace_attach() until the task has // entered the ptrace-stop (or exited) via JOBCTL_TRAPPING. } target.tg.signalHandlers.mu.Unlock() diff --git a/pkg/sentry/kernel/rseq.go b/pkg/sentry/kernel/rseq.go index 0a954bc16..6d3314e81 100644 --- a/pkg/sentry/kernel/rseq.go +++ b/pkg/sentry/kernel/rseq.go @@ -66,7 +66,7 @@ func (t *Task) SetRSEQCriticalRegion(rscr RSEQCriticalRegion) error { if rscr.CriticalSection.Contains(rscr.Restart) { return syserror.EINVAL } - // TODO: check that rscr.CriticalSection and rscr.Restart are in + // TODO(jamieliu): check that rscr.CriticalSection and rscr.Restart are in // the application address range, for consistency with Linux t.tg.rscr.Store(&rscr) return nil diff --git a/pkg/sentry/kernel/sched/cpuset.go b/pkg/sentry/kernel/sched/cpuset.go index 69aee9127..41ac1067d 100644 --- a/pkg/sentry/kernel/sched/cpuset.go +++ b/pkg/sentry/kernel/sched/cpuset.go @@ -29,7 +29,7 @@ type CPUSet []byte // CPUSetSize returns the size in bytes of a CPUSet that can contain num cpus. func CPUSetSize(num uint) uint { - // NOTE: Applications may expect that the size of a CPUSet in + // NOTE(b/68859821): Applications may expect that the size of a CPUSet in // bytes is always a multiple of sizeof(unsigned long), since this is true // in Linux. Thus we always round up. bytes := (num + bitsPerByte - 1) / bitsPerByte diff --git a/pkg/sentry/kernel/semaphore/semaphore.go b/pkg/sentry/kernel/semaphore/semaphore.go index 29a2eb804..2b7c1a9bc 100644 --- a/pkg/sentry/kernel/semaphore/semaphore.go +++ b/pkg/sentry/kernel/semaphore/semaphore.go @@ -302,7 +302,7 @@ func (s *Set) SetVal(ctx context.Context, num int32, val int16, creds *auth.Cred return syserror.ERANGE } - // TODO: Clear undo entries in all processes + // TODO(b/29354920): Clear undo entries in all processes sem.value = val sem.pid = pid s.changeTime = ktime.NowFromContext(ctx) @@ -336,7 +336,7 @@ func (s *Set) SetValAll(ctx context.Context, vals []uint16, creds *auth.Credenti for i, val := range vals { sem := &s.sems[i] - // TODO: Clear undo entries in all processes + // TODO(b/29354920): Clear undo entries in all processes sem.value = int16(val) sem.pid = pid sem.wakeWaiters() @@ -481,7 +481,7 @@ func (s *Set) executeOps(ctx context.Context, ops []linux.Sembuf, pid int32) (ch } // All operations succeeded, apply them. - // TODO: handle undo operations. + // TODO(b/29354920): handle undo operations. for i, v := range tmpVals { s.sems[i].value = v s.sems[i].wakeWaiters() diff --git a/pkg/sentry/kernel/shm/shm.go b/pkg/sentry/kernel/shm/shm.go index 349f2a26e..d4812a065 100644 --- a/pkg/sentry/kernel/shm/shm.go +++ b/pkg/sentry/kernel/shm/shm.go @@ -427,7 +427,7 @@ func (s *Shm) AddMapping(ctx context.Context, _ memmap.MappingSpace, _ usermem.A func (s *Shm) RemoveMapping(ctx context.Context, _ memmap.MappingSpace, _ usermem.AddrRange, _ uint64, _ bool) { s.mu.Lock() defer s.mu.Unlock() - // TODO: RemoveMapping may be called during task exit, when ctx + // TODO(b/38173783): RemoveMapping may be called during task exit, when ctx // is context.Background. Gracefully handle missing clocks. Failing to // update the detach time in these cases is ok, since no one can observe the // omission. diff --git a/pkg/sentry/kernel/syscalls.go b/pkg/sentry/kernel/syscalls.go index 7eb99718d..293b21249 100644 --- a/pkg/sentry/kernel/syscalls.go +++ b/pkg/sentry/kernel/syscalls.go @@ -165,7 +165,7 @@ type Stracer interface { // // The returned private data is passed to SyscallExit. // - // TODO: remove kernel imports from the strace + // TODO(gvisor.dev/issue/155): remove kernel imports from the strace // package so that the type can be used directly. SyscallEnter(t *Task, sysno uintptr, args arch.SyscallArguments, flags uint32) interface{} diff --git a/pkg/sentry/kernel/task_context.go b/pkg/sentry/kernel/task_context.go index 1b4d4cf2f..ac38dd157 100644 --- a/pkg/sentry/kernel/task_context.go +++ b/pkg/sentry/kernel/task_context.go @@ -60,7 +60,7 @@ func (tc *TaskContext) release() { // Nil out pointers so that if the task is saved after release, it doesn't // follow the pointers to possibly now-invalid objects. if tc.MemoryManager != nil { - // TODO + // TODO(b/38173783) tc.MemoryManager.DecUsers(context.Background()) tc.MemoryManager = nil } diff --git a/pkg/sentry/kernel/task_exec.go b/pkg/sentry/kernel/task_exec.go index 9fca90a1c..b49f902a5 100644 --- a/pkg/sentry/kernel/task_exec.go +++ b/pkg/sentry/kernel/task_exec.go @@ -208,7 +208,7 @@ func (r *runSyscallAfterExecStop) execute(t *Task) taskRunState { t.tc = *r.tc t.mu.Unlock() t.unstopVforkParent() - // NOTE: All locks must be dropped prior to calling Activate. + // NOTE(b/30316266): All locks must be dropped prior to calling Activate. t.MemoryManager().Activate() t.ptraceExec(oldTID) diff --git a/pkg/sentry/kernel/task_exit.go b/pkg/sentry/kernel/task_exit.go index 1a0734ab6..a07956208 100644 --- a/pkg/sentry/kernel/task_exit.go +++ b/pkg/sentry/kernel/task_exit.go @@ -339,7 +339,7 @@ func (t *Task) exitChildren() { }, true /* group */) other.signalHandlers.mu.Unlock() } - // TODO: The init process waits for all processes in the + // TODO(b/37722272): The init process waits for all processes in the // namespace to exit before completing its own exit // (kernel/pid_namespace.c:zap_pid_ns_processes()). Stop until all // other tasks in the namespace are dead, except possibly for this @@ -692,7 +692,7 @@ func (t *Task) exitNotificationSignal(sig linux.Signal, receiver *Task) *arch.Si info.Code = arch.CLD_EXITED info.SetStatus(int32(t.exitStatus.Code)) } - // TODO: Set utime, stime. + // TODO(b/72102453): Set utime, stime. return info } diff --git a/pkg/sentry/kernel/task_identity.go b/pkg/sentry/kernel/task_identity.go index e105eba13..6c9608f8d 100644 --- a/pkg/sentry/kernel/task_identity.go +++ b/pkg/sentry/kernel/task_identity.go @@ -421,7 +421,7 @@ func (t *Task) SetKeepCaps(k bool) { // updateCredsForExec updates t.creds to reflect an execve(). // -// NOTE: We currently do not implement privileged executables +// NOTE(b/30815691): We currently do not implement privileged executables // (set-user/group-ID bits and file capabilities). This allows us to make a lot // of simplifying assumptions: // diff --git a/pkg/sentry/kernel/task_run.go b/pkg/sentry/kernel/task_run.go index 6b5fe7165..7115aa967 100644 --- a/pkg/sentry/kernel/task_run.go +++ b/pkg/sentry/kernel/task_run.go @@ -110,7 +110,7 @@ func (t *Task) doStop() { return } t.Deactivate() - // NOTE: t.Activate() must be called without any locks held, so + // NOTE(b/30316266): t.Activate() must be called without any locks held, so // this defer must precede the defer for unlocking the signal mutex. defer t.Activate() t.accountTaskGoroutineEnter(TaskGoroutineStopped) diff --git a/pkg/sentry/kernel/task_signals.go b/pkg/sentry/kernel/task_signals.go index 3a8e61900..7f2e0df72 100644 --- a/pkg/sentry/kernel/task_signals.go +++ b/pkg/sentry/kernel/task_signals.go @@ -509,7 +509,7 @@ func (t *Task) canReceiveSignalLocked(sig linux.Signal) bool { if t.stop != nil { return false } - // - TODO: No special case for when t is also the sending task, + // - TODO(b/38173783): No special case for when t is also the sending task, // because the identity of the sender is unknown. // - Do not choose tasks that have already been interrupted, as they may be // busy handling another signal. @@ -895,7 +895,7 @@ func (t *Task) signalStop(target *Task, code int32, status int32) { sigchld.SetPid(int32(t.tg.pidns.tids[target])) sigchld.SetUid(int32(target.Credentials().RealKUID.In(t.UserNamespace()).OrOverflow())) sigchld.SetStatus(status) - // TODO: Set utime, stime. + // TODO(b/72102453): Set utime, stime. t.sendSignalLocked(sigchld, true /* group */) } } diff --git a/pkg/sentry/kernel/task_stop.go b/pkg/sentry/kernel/task_stop.go index 36846484c..1302cadc1 100644 --- a/pkg/sentry/kernel/task_stop.go +++ b/pkg/sentry/kernel/task_stop.go @@ -69,7 +69,7 @@ import ( // A TaskStop is a condition visible to the task control flow graph that // prevents a task goroutine from running or exiting, i.e. an internal stop. // -// NOTE: Most TaskStops don't contain any data; they're +// NOTE(b/30793614): Most TaskStops don't contain any data; they're // distinguished by their type. The obvious way to implement such a TaskStop // is: // diff --git a/pkg/sentry/loader/loader.go b/pkg/sentry/loader/loader.go index 80ad59dde..79051befa 100644 --- a/pkg/sentry/loader/loader.go +++ b/pkg/sentry/loader/loader.go @@ -70,7 +70,7 @@ func openPath(ctx context.Context, mm *fs.MountNamespace, root, wd *fs.Dirent, m defer d.DecRef() perms := fs.PermMask{ - // TODO: Linux requires only execute + // TODO(gvisor.dev/issue/160): Linux requires only execute // permission, not read. However, our backing filesystems may // prevent us from reading the file without read permission. // diff --git a/pkg/sentry/loader/vdso.go b/pkg/sentry/loader/vdso.go index 18b7e90d8..8c196df84 100644 --- a/pkg/sentry/loader/vdso.go +++ b/pkg/sentry/loader/vdso.go @@ -194,7 +194,7 @@ func validateVDSO(ctx context.Context, f *fs.File, size uint64) (elfInfo, error) // VDSO describes a VDSO. // -// NOTE: to support multiple architectures or operating systems, this +// NOTE(mpratt): to support multiple architectures or operating systems, this // would need to contain a VDSO for each. // // +stateify savable @@ -262,7 +262,7 @@ func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { return &VDSO{ ParamPage: mm.NewSpecialMappable("[vvar]", mfp, paramPage), - // TODO: Don't advertise the VDSO, as + // TODO(gvisor.dev/issue/157): Don't advertise the VDSO, as // some applications may not be able to handle multiple [vdso] // hints. vdso: mm.NewSpecialMappable("", mfp, vdso), @@ -279,7 +279,7 @@ func PrepareVDSO(mfp pgalloc.MemoryFileProvider) (*VDSO, error) { // kernel simply directly maps the entire file into process memory, with very // little real ELF parsing. // -// NOTE: This means that userspace can, and unfortunately does, +// NOTE(b/25323870): This means that userspace can, and unfortunately does, // depend on parts of the ELF that would normally not be mapped. To maintain // compatibility with such binaries, we load the VDSO much like Linux. // diff --git a/pkg/sentry/memmap/memmap.go b/pkg/sentry/memmap/memmap.go index 1ef1f0dd8..3f6f7ebd0 100644 --- a/pkg/sentry/memmap/memmap.go +++ b/pkg/sentry/memmap/memmap.go @@ -356,6 +356,6 @@ type MMapOpts struct { // Hint is the name used for the mapping in /proc/[pid]/maps. If Hint is // empty, MappingIdentity.MappedName() will be used instead. // - // TODO: Replace entirely with MappingIdentity? + // TODO(jamieliu): Replace entirely with MappingIdentity? Hint string } diff --git a/pkg/sentry/mm/aio_context.go b/pkg/sentry/mm/aio_context.go index f7ff06de0..7075792e0 100644 --- a/pkg/sentry/mm/aio_context.go +++ b/pkg/sentry/mm/aio_context.go @@ -331,7 +331,7 @@ func (mm *MemoryManager) NewAIOContext(ctx context.Context, events uint32) (uint Length: aioRingBufferSize, MappingIdentity: m, Mappable: m, - // TODO: Linux does "do_mmap_pgoff(..., PROT_READ | + // TODO(fvoznika): Linux does "do_mmap_pgoff(..., PROT_READ | // PROT_WRITE, ...)" in fs/aio.c:aio_setup_ring(); why do we make this // mapping read-only? Perms: usermem.Read, diff --git a/pkg/sentry/mm/procfs.go b/pkg/sentry/mm/procfs.go index 0c4b8895d..7cdbf6e25 100644 --- a/pkg/sentry/mm/procfs.go +++ b/pkg/sentry/mm/procfs.go @@ -69,7 +69,7 @@ func (mm *MemoryManager) ReadMapsSeqFileData(ctx context.Context, handle seqfile start = *handle.(*usermem.Addr) } for vseg := mm.vmas.LowerBoundSegment(start); vseg.Ok(); vseg = vseg.NextSegment() { - // FIXME: If we use a usermem.Addr for the handle, we get + // FIXME(b/30793614): If we use a usermem.Addr for the handle, we get // "panic: autosave error: type usermem.Addr is not registered". vmaAddr := vseg.End() data = append(data, seqfile.SeqData{ @@ -88,7 +88,7 @@ func (mm *MemoryManager) ReadMapsSeqFileData(ctx context.Context, handle seqfile // // Artifically adjust the seqfile handle so we only output vsyscall entry once. if start != vsyscallEnd { - // FIXME: Can't get a pointer to constant vsyscallEnd. + // FIXME(b/30793614): Can't get a pointer to constant vsyscallEnd. vmaAddr := vsyscallEnd data = append(data, seqfile.SeqData{ Buf: []byte(vsyscallMapsEntry), @@ -134,7 +134,7 @@ func (mm *MemoryManager) appendVMAMapsEntryLocked(ctx context.Context, vseg vmaI if vma.hint != "" { s = vma.hint } else if vma.id != nil { - // FIXME: We are holding mm.mappingMu here, which is + // FIXME(jamieliu): We are holding mm.mappingMu here, which is // consistent with Linux's holding mmap_sem in // fs/proc/task_mmu.c:show_map_vma() => fs/seq_file.c:seq_file_path(). // However, it's not clear that fs.File.MappedName() is actually @@ -162,7 +162,7 @@ func (mm *MemoryManager) ReadSmapsSeqFileData(ctx context.Context, handle seqfil start = *handle.(*usermem.Addr) } for vseg := mm.vmas.LowerBoundSegment(start); vseg.Ok(); vseg = vseg.NextSegment() { - // FIXME: If we use a usermem.Addr for the handle, we get + // FIXME(b/30793614): If we use a usermem.Addr for the handle, we get // "panic: autosave error: type usermem.Addr is not registered". vmaAddr := vseg.End() data = append(data, seqfile.SeqData{ @@ -174,7 +174,7 @@ func (mm *MemoryManager) ReadSmapsSeqFileData(ctx context.Context, handle seqfil // We always emulate vsyscall, so advertise it here. See // ReadMapsSeqFileData for additional commentary. if start != vsyscallEnd { - // FIXME: Can't get a pointer to constant vsyscallEnd. + // FIXME(b/30793614): Can't get a pointer to constant vsyscallEnd. vmaAddr := vsyscallEnd data = append(data, seqfile.SeqData{ Buf: []byte(vsyscallSmapsEntry), diff --git a/pkg/sentry/mm/special_mappable.go b/pkg/sentry/mm/special_mappable.go index cfbf7a104..3b5161998 100644 --- a/pkg/sentry/mm/special_mappable.go +++ b/pkg/sentry/mm/special_mappable.go @@ -136,7 +136,7 @@ func (m *SpecialMappable) Length() uint64 { // NewSharedAnonMappable returns a SpecialMappable that implements the // semantics of mmap(MAP_SHARED|MAP_ANONYMOUS) and mappings of /dev/zero. // -// TODO: The use of SpecialMappable is a lazy code reuse hack. Linux +// TODO(jamieliu): The use of SpecialMappable is a lazy code reuse hack. Linux // uses an ephemeral file created by mm/shmem.c:shmem_zero_setup(); we should // do the same to get non-zero device and inode IDs. func NewSharedAnonMappable(length uint64, mfp pgalloc.MemoryFileProvider) (*SpecialMappable, error) { diff --git a/pkg/sentry/mm/syscalls.go b/pkg/sentry/mm/syscalls.go index cc7eb76d2..7b675b9b5 100644 --- a/pkg/sentry/mm/syscalls.go +++ b/pkg/sentry/mm/syscalls.go @@ -137,7 +137,7 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (userme return 0, err } - // TODO: In Linux, VM_LOCKONFAULT (which may be set on the new + // TODO(jamieliu): In Linux, VM_LOCKONFAULT (which may be set on the new // vma by mlockall(MCL_FUTURE|MCL_ONFAULT) => mm_struct::def_flags) appears // to effectively disable MAP_POPULATE by unsetting FOLL_POPULATE in // mm/util.c:vm_mmap_pgoff() => mm/gup.c:__mm_populate() => @@ -148,7 +148,7 @@ func (mm *MemoryManager) MMap(ctx context.Context, opts memmap.MMapOpts) (userme mm.populateVMAAndUnlock(ctx, vseg, ar, true) case opts.Mappable == nil && length <= privateAllocUnit: - // NOTE: Get pmas and map eagerly in the hope + // NOTE(b/63077076, b/63360184): Get pmas and map eagerly in the hope // that doing so will save on future page faults. We only do this for // anonymous mappings, since otherwise the cost of // memmap.Mappable.Translate is unknown; and only for small mappings, @@ -698,7 +698,7 @@ func (mm *MemoryManager) Brk(ctx context.Context, addr usermem.Addr) (usermem.Ad return mm.brk.End, syserror.EINVAL } - // TODO: This enforces RLIMIT_DATA, but is + // TODO(gvisor.dev/issue/156): This enforces RLIMIT_DATA, but is // slightly more permissive than the usual data limit. In particular, // this only limits the size of the heap; a true RLIMIT_DATA limits the // size of heap + data + bss. The segment sizes need to be plumbed from diff --git a/pkg/sentry/mm/vma.go b/pkg/sentry/mm/vma.go index e9c9a80ea..931995254 100644 --- a/pkg/sentry/mm/vma.go +++ b/pkg/sentry/mm/vma.go @@ -274,7 +274,7 @@ func (mm *MemoryManager) getVMAsLocked(ctx context.Context, ar usermem.AddrRange // Loop invariants: vgap = vseg.PrevGap(); addr < vseg.End(). vma := vseg.ValuePtr() if addr < vseg.Start() { - // TODO: Implement vma.growsDown here. + // TODO(jamieliu): Implement vma.growsDown here. return vbegin, vgap, syserror.EFAULT } diff --git a/pkg/sentry/platform/kvm/kvm_amd64_unsafe.go b/pkg/sentry/platform/kvm/kvm_amd64_unsafe.go index c0a0af92d..d0f6bb225 100644 --- a/pkg/sentry/platform/kvm/kvm_amd64_unsafe.go +++ b/pkg/sentry/platform/kvm/kvm_amd64_unsafe.go @@ -62,7 +62,7 @@ func updateSystemValues(fd int) error { // Calculate whether guestPCID is supported. // - // FIXME: These should go through the much more pleasant + // FIXME(ascannell): These should go through the much more pleasant // cpuid package interfaces, once a way to accept raw kvm CPUID entries // is plumbed (or some rough equivalent). for i := 0; i < int(cpuidSupported.nr); i++ { diff --git a/pkg/sentry/platform/platform.go b/pkg/sentry/platform/platform.go index d1c9458ea..0e48417b9 100644 --- a/pkg/sentry/platform/platform.go +++ b/pkg/sentry/platform/platform.go @@ -181,7 +181,7 @@ var ( // this signal both to Contexts and to the sentry itself, under the assumption // that they originate from races with Context.Interrupt(). // -// NOTE: The Go runtime only guarantees that a small subset +// NOTE(b/23420492): The Go runtime only guarantees that a small subset // of signals will be always be unblocked on all threads, one of which // is SIGCHLD. const SignalInterrupt = linux.SIGCHLD diff --git a/pkg/sentry/platform/ptrace/subprocess.go b/pkg/sentry/platform/ptrace/subprocess.go index 82f125073..2a5d699ec 100644 --- a/pkg/sentry/platform/ptrace/subprocess.go +++ b/pkg/sentry/platform/ptrace/subprocess.go @@ -79,7 +79,7 @@ func (tp *threadPool) lookupOrCreate(currentTID int32, newThread func() *thread) // Before creating a new thread, see if we can find a thread // whose system tid has disappeared. // - // TODO: Other parts of this package depend on + // TODO(b/77216482): Other parts of this package depend on // threads never exiting. for origTID, t := range tp.threads { // Signal zero is an easy existence check. diff --git a/pkg/sentry/platform/ring0/x86.go b/pkg/sentry/platform/ring0/x86.go index 7c88010d8..4c6daec22 100644 --- a/pkg/sentry/platform/ring0/x86.go +++ b/pkg/sentry/platform/ring0/x86.go @@ -116,7 +116,7 @@ const ( // // Note that sign-extension semantics apply to the highest order bit. // -// FIXME: This should use the cpuid passed to Init. +// FIXME(b/69382326): This should use the cpuid passed to Init. func VirtualAddressBits() uint32 { ax, _, _, _ := cpuid.HostID(0x80000008, 0) return (ax >> 8) & 0xff @@ -124,7 +124,7 @@ func VirtualAddressBits() uint32 { // PhysicalAddressBits returns the number of bits available for physical addresses. // -// FIXME: This should use the cpuid passed to Init. +// FIXME(b/69382326): This should use the cpuid passed to Init. func PhysicalAddressBits() uint32 { ax, _, _, _ := cpuid.HostID(0x80000008, 0) return ax & 0xff diff --git a/pkg/sentry/sighandling/sighandling.go b/pkg/sentry/sighandling/sighandling.go index 6b5d5f993..571245ce5 100644 --- a/pkg/sentry/sighandling/sighandling.go +++ b/pkg/sentry/sighandling/sighandling.go @@ -86,7 +86,7 @@ func handleSignals(sigchans []chan os.Signal, handler func(linux.Signal), start, // // Otherwise ignore the signal. // - // TODO: Drop in Go 1.12, which uses tgkill + // TODO(b/114489875): Drop in Go 1.12, which uses tgkill // in runtime.raise. switch signal { case linux.SIGHUP, linux.SIGINT, linux.SIGTERM: diff --git a/pkg/sentry/sighandling/sighandling_unsafe.go b/pkg/sentry/sighandling/sighandling_unsafe.go index 5913d47a8..db6e71487 100644 --- a/pkg/sentry/sighandling/sighandling_unsafe.go +++ b/pkg/sentry/sighandling/sighandling_unsafe.go @@ -23,7 +23,7 @@ import ( "gvisor.googlesource.com/gvisor/pkg/abi/linux" ) -// TODO: Move to pkg/abi/linux along with definitions in +// TODO(b/34161764): Move to pkg/abi/linux along with definitions in // pkg/sentry/arch. type sigaction struct { handler uintptr diff --git a/pkg/sentry/socket/epsocket/epsocket.go b/pkg/sentry/socket/epsocket/epsocket.go index 23138d874..768fa0dfa 100644 --- a/pkg/sentry/socket/epsocket/epsocket.go +++ b/pkg/sentry/socket/epsocket/epsocket.go @@ -608,7 +608,7 @@ func (s *SocketOperations) Shutdown(t *kernel.Task, how int) *syserr.Error { // GetSockOpt implements the linux syscall getsockopt(2) for sockets backed by // tcpip.Endpoint. func (s *SocketOperations) GetSockOpt(t *kernel.Task, level, name, outLen int) (interface{}, *syserr.Error) { - // TODO: Unlike other socket options, SO_TIMESTAMP is + // TODO(b/78348848): Unlike other socket options, SO_TIMESTAMP is // implemented specifically for epsocket.SocketOperations rather than // commonEndpoint. commonEndpoint should be extended to support socket // options where the implementation is not shared, as unix sockets need @@ -658,7 +658,7 @@ func GetSockOpt(t *kernel.Task, s socket.Socket, ep commonEndpoint, family int, // getSockOptSocket implements GetSockOpt when level is SOL_SOCKET. func getSockOptSocket(t *kernel.Task, s socket.Socket, ep commonEndpoint, family int, skType transport.SockType, name, outLen int) (interface{}, *syserr.Error) { - // TODO: Stop rejecting short optLen values in getsockopt. + // TODO(b/124056281): Stop rejecting short optLen values in getsockopt. switch name { case linux.SO_TYPE: if outLen < sizeOfInt32 { @@ -789,7 +789,7 @@ func getSockOptSocket(t *kernel.Task, s socket.Socket, ep commonEndpoint, family return linux.Linger{}, nil case linux.SO_SNDTIMEO: - // TODO: Linux allows shorter lengths for partial results. + // TODO(igudger): Linux allows shorter lengths for partial results. if outLen < linux.SizeOfTimeval { return nil, syserr.ErrInvalidArgument } @@ -797,7 +797,7 @@ func getSockOptSocket(t *kernel.Task, s socket.Socket, ep commonEndpoint, family return linux.NsecToTimeval(s.SendTimeout()), nil case linux.SO_RCVTIMEO: - // TODO: Linux allows shorter lengths for partial results. + // TODO(igudger): Linux allows shorter lengths for partial results. if outLen < linux.SizeOfTimeval { return nil, syserr.ErrInvalidArgument } @@ -894,7 +894,7 @@ func getSockOptTCP(t *kernel.Task, ep commonEndpoint, name, outLen int) (interfa return nil, syserr.TranslateNetstackError(err) } - // TODO: Translate fields once they are added to + // TODO(b/64800844): Translate fields once they are added to // tcpip.TCPInfoOption. info := linux.TCPInfo{} @@ -995,7 +995,7 @@ func getSockOptIP(t *kernel.Task, ep commonEndpoint, name, outLen int) (interfac // SetSockOpt implements the linux syscall setsockopt(2) for sockets backed by // tcpip.Endpoint. func (s *SocketOperations) SetSockOpt(t *kernel.Task, level int, name int, optVal []byte) *syserr.Error { - // TODO: Unlike other socket options, SO_TIMESTAMP is + // TODO(b/78348848): Unlike other socket options, SO_TIMESTAMP is // implemented specifically for epsocket.SocketOperations rather than // commonEndpoint. commonEndpoint should be extended to support socket // options where the implementation is not shared, as unix sockets need @@ -1338,7 +1338,7 @@ func setSockOptIP(t *kernel.Task, ep commonEndpoint, name int, optVal []byte) *s return syserr.TranslateNetstackError(ep.SetSockOpt(tcpip.AddMembershipOption{ NIC: tcpip.NICID(req.InterfaceIndex), - // TODO: Change AddMembership to use the standard + // TODO(igudger): Change AddMembership to use the standard // any address representation. InterfaceAddr: tcpip.Address(req.InterfaceAddr[:]), MulticastAddr: tcpip.Address(req.MulticastAddr[:]), @@ -1352,7 +1352,7 @@ func setSockOptIP(t *kernel.Task, ep commonEndpoint, name int, optVal []byte) *s return syserr.TranslateNetstackError(ep.SetSockOpt(tcpip.RemoveMembershipOption{ NIC: tcpip.NICID(req.InterfaceIndex), - // TODO: Change DropMembership to use the standard + // TODO(igudger): Change DropMembership to use the standard // any address representation. InterfaceAddr: tcpip.Address(req.InterfaceAddr[:]), MulticastAddr: tcpip.Address(req.MulticastAddr[:]), @@ -1380,7 +1380,7 @@ func setSockOptIP(t *kernel.Task, ep commonEndpoint, name int, optVal []byte) *s )) case linux.MCAST_JOIN_GROUP: - // FIXME: Implement MCAST_JOIN_GROUP. + // FIXME(b/124219304): Implement MCAST_JOIN_GROUP. t.Kernel().EmitUnimplementedEvent(t) return syserr.ErrInvalidArgument @@ -1695,7 +1695,7 @@ func (s *SocketOperations) coalescingRead(ctx context.Context, dst usermem.IOSeq // nonBlockingRead issues a non-blocking read. // -// TODO: Support timestamps for stream sockets. +// TODO(b/78348848): Support timestamps for stream sockets. func (s *SocketOperations) nonBlockingRead(ctx context.Context, dst usermem.IOSequence, peek, trunc, senderRequested bool) (int, int, interface{}, uint32, socket.ControlMessages, *syserr.Error) { isPacket := s.isPacketBased() @@ -1762,7 +1762,7 @@ func (s *SocketOperations) nonBlockingRead(ctx context.Context, dst usermem.IOSe dst = dst.DropFirst(n) num, err := dst.CopyOutFrom(ctx, safemem.FromVecReaderFunc{func(dsts [][]byte) (int64, error) { n, _, err := s.Endpoint.Peek(dsts) - // TODO: Handle peek timestamp. + // TODO(b/78348848): Handle peek timestamp. if err != nil { return int64(n), syserr.TranslateNetstackError(err).ToError() } @@ -1963,7 +1963,7 @@ func (s *SocketOperations) SendMsg(t *kernel.Task, src usermem.IOSequence, to [] func (s *SocketOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { // SIOCGSTAMP is implemented by epsocket rather than all commonEndpoint // sockets. - // TODO: Add a commonEndpoint method to support SIOCGSTAMP. + // TODO(b/78348848): Add a commonEndpoint method to support SIOCGSTAMP. if int(args[1].Int()) == syscall.SIOCGSTAMP { s.readMu.Lock() defer s.readMu.Unlock() @@ -2153,19 +2153,19 @@ func interfaceIoctl(ctx context.Context, io usermem.IO, arg int, ifr *linux.IFRe case syscall.SIOCGIFMAP: // Gets the hardware parameters of the device. - // TODO: Implement. + // TODO(b/71872867): Implement. case syscall.SIOCGIFTXQLEN: // Gets the transmit queue length of the device. - // TODO: Implement. + // TODO(b/71872867): Implement. case syscall.SIOCGIFDSTADDR: // Gets the destination address of a point-to-point device. - // TODO: Implement. + // TODO(b/71872867): Implement. case syscall.SIOCGIFBRDADDR: // Gets the broadcast address of a device. - // TODO: Implement. + // TODO(b/71872867): Implement. case syscall.SIOCGIFNETMASK: // Gets the network mask of a device. diff --git a/pkg/sentry/socket/epsocket/save_restore.go b/pkg/sentry/socket/epsocket/save_restore.go index 34d9a7cf0..f19afb6c0 100644 --- a/pkg/sentry/socket/epsocket/save_restore.go +++ b/pkg/sentry/socket/epsocket/save_restore.go @@ -20,7 +20,7 @@ import ( // afterLoad is invoked by stateify. func (s *Stack) afterLoad() { - s.Stack = stack.StackFromEnv // FIXME + s.Stack = stack.StackFromEnv // FIXME(b/36201077) if s.Stack == nil { panic("can't restore without netstack/tcpip/stack.Stack") } diff --git a/pkg/sentry/socket/epsocket/stack.go b/pkg/sentry/socket/epsocket/stack.go index c0081c819..37c48f4bc 100644 --- a/pkg/sentry/socket/epsocket/stack.go +++ b/pkg/sentry/socket/epsocket/stack.go @@ -77,7 +77,7 @@ func (s *Stack) InterfaceAddrs() map[int32][]inet.InterfaceAddr { Family: family, PrefixLen: uint8(len(a.Address) * 8), Addr: []byte(a.Address), - // TODO: Other fields. + // TODO(b/68878065): Other fields. }) } nicAddrs[int32(id)] = addrs diff --git a/pkg/sentry/socket/hostinet/socket.go b/pkg/sentry/socket/hostinet/socket.go index c4848b313..49349074f 100644 --- a/pkg/sentry/socket/hostinet/socket.go +++ b/pkg/sentry/socket/hostinet/socket.go @@ -348,7 +348,7 @@ func (s *socketOperations) SetSockOpt(t *kernel.Task, level int, name int, opt [ func (s *socketOperations) RecvMsg(t *kernel.Task, dst usermem.IOSequence, flags int, haveDeadline bool, deadline ktime.Time, senderRequested bool, controlDataLen uint64) (int, int, interface{}, uint32, socket.ControlMessages, *syserr.Error) { // Whitelist flags. // - // FIXME: We can't support MSG_ERRQUEUE because it uses ancillary + // FIXME(jamieliu): We can't support MSG_ERRQUEUE because it uses ancillary // messages that netstack/tcpip/transport/unix doesn't understand. Kill the // Socket interface's dependence on netstack. if flags&^(syscall.MSG_DONTWAIT|syscall.MSG_PEEK|syscall.MSG_TRUNC) != 0 { diff --git a/pkg/sentry/socket/netlink/route/protocol.go b/pkg/sentry/socket/netlink/route/protocol.go index 7e70b09b2..e414b829b 100644 --- a/pkg/sentry/socket/netlink/route/protocol.go +++ b/pkg/sentry/socket/netlink/route/protocol.go @@ -110,7 +110,7 @@ func (p *Protocol) dumpLinks(ctx context.Context, hdr linux.NetlinkMessageHeader m.PutAttr(linux.IFLA_ADDRESS, mac) m.PutAttr(linux.IFLA_BROADCAST, brd) - // TODO: There are many more attributes. + // TODO(b/68878065): There are many more attributes. } return nil @@ -122,7 +122,7 @@ func (p *Protocol) dumpAddrs(ctx context.Context, hdr linux.NetlinkMessageHeader // netlink header and 1 byte protocol family common to all // NETLINK_ROUTE requests. // - // TODO: Filter output by passed protocol family. + // TODO(b/68878065): Filter output by passed protocol family. // The RTM_GETADDR dump response is a set of RTM_NEWADDR messages each // containing an InterfaceAddrMessage followed by a set of netlink @@ -151,7 +151,7 @@ func (p *Protocol) dumpAddrs(ctx context.Context, hdr linux.NetlinkMessageHeader m.PutAttr(linux.IFA_ADDRESS, []byte(a.Addr)) - // TODO: There are many more attributes. + // TODO(b/68878065): There are many more attributes. } } @@ -175,7 +175,7 @@ func (p *Protocol) ProcessMessage(ctx context.Context, hdr linux.NetlinkMessageH } } - // TODO: Only the dump variant of the types below are + // TODO(b/68878065): Only the dump variant of the types below are // supported. if hdr.Flags&linux.NLM_F_DUMP != linux.NLM_F_DUMP { return syserr.ErrNotSupported diff --git a/pkg/sentry/socket/netlink/socket.go b/pkg/sentry/socket/netlink/socket.go index 0fe9b39b6..a34f9d3ca 100644 --- a/pkg/sentry/socket/netlink/socket.go +++ b/pkg/sentry/socket/netlink/socket.go @@ -168,7 +168,7 @@ func (s *Socket) EventUnregister(e *waiter.Entry) { // Ioctl implements fs.FileOperations.Ioctl. func (s *Socket) Ioctl(ctx context.Context, io usermem.IO, args arch.SyscallArguments) (uintptr, error) { - // TODO: no ioctls supported. + // TODO(b/68878065): no ioctls supported. return 0, syserror.ENOTTY } @@ -319,7 +319,7 @@ func (s *Socket) GetSockOpt(t *kernel.Task, level int, name int, outLen int) (in t.Kernel().EmitUnimplementedEvent(t) } } - // TODO: other sockopts are not supported. + // TODO(b/68878065): other sockopts are not supported. return nil, syserr.ErrProtocolNotAvailable } @@ -369,7 +369,7 @@ func (s *Socket) SetSockOpt(t *kernel.Task, level int, name int, opt []byte) *sy } } - // TODO: other sockopts are not supported. + // TODO(b/68878065): other sockopts are not supported. return syserr.ErrProtocolNotAvailable } @@ -389,7 +389,7 @@ func (s *Socket) GetSockName(t *kernel.Task) (interface{}, uint32, *syserr.Error func (s *Socket) GetPeerName(t *kernel.Task) (interface{}, uint32, *syserr.Error) { sa := linux.SockAddrNetlink{ Family: linux.AF_NETLINK, - // TODO: Support non-kernel peers. For now the peer + // TODO(b/68878065): Support non-kernel peers. For now the peer // must be the kernel. PortID: 0, } @@ -540,7 +540,7 @@ func (s *Socket) processMessages(ctx context.Context, buf []byte) *syserr.Error continue } - // TODO: ACKs not supported yet. + // TODO(b/68877377): ACKs not supported yet. if hdr.Flags&linux.NLM_F_ACK == linux.NLM_F_ACK { return syserr.ErrNotSupported } diff --git a/pkg/sentry/socket/rpcinet/conn/conn.go b/pkg/sentry/socket/rpcinet/conn/conn.go index 9c749b888..64106c4b5 100644 --- a/pkg/sentry/socket/rpcinet/conn/conn.go +++ b/pkg/sentry/socket/rpcinet/conn/conn.go @@ -50,7 +50,7 @@ type RPCConnection struct { // NewRPCConnection initializes a RPC connection to a socket gofer. func NewRPCConnection(s *unet.Socket) *RPCConnection { conn := &RPCConnection{socket: s, requests: map[uint64]request{}} - go func() { // S/R-FIXME + go func() { // S/R-FIXME(b/77962828) var nums [16]byte for { for n := 0; n < len(nums); { diff --git a/pkg/sentry/socket/rpcinet/notifier/notifier.go b/pkg/sentry/socket/rpcinet/notifier/notifier.go index d9bda78b0..f06d12231 100644 --- a/pkg/sentry/socket/rpcinet/notifier/notifier.go +++ b/pkg/sentry/socket/rpcinet/notifier/notifier.go @@ -64,7 +64,7 @@ func NewRPCNotifier(cn *conn.RPCConnection) (*Notifier, error) { fdMap: make(map[uint32]*fdInfo), } - go w.waitAndNotify() // S/R-FIXME + go w.waitAndNotify() // S/R-FIXME(b/77962828) return w, nil } @@ -166,7 +166,7 @@ func (n *Notifier) waitAndNotify() error { res := n.rpcConn.Request(id).Result.(*pb.SyscallResponse_EpollWait).EpollWait.Result if e, ok := res.(*pb.EpollWaitResponse_ErrorNumber); ok { err := syscall.Errno(e.ErrorNumber) - // NOTE: I don't think epoll_wait can return EAGAIN but I'm being + // NOTE(magi): I don't think epoll_wait can return EAGAIN but I'm being // conseratively careful here since exiting the notification thread // would be really bad. if err == syscall.EINTR || err == syscall.EAGAIN { diff --git a/pkg/sentry/socket/rpcinet/socket.go b/pkg/sentry/socket/rpcinet/socket.go index 3418a6d75..cf8f69efb 100644 --- a/pkg/sentry/socket/rpcinet/socket.go +++ b/pkg/sentry/socket/rpcinet/socket.go @@ -288,7 +288,7 @@ func (s *socketOperations) Accept(t *kernel.Task, peerRequested bool, flags int, if blocking && se == syserr.ErrTryAgain { // Register for notifications. e, ch := waiter.NewChannelEntry(nil) - // FIXME: This waiter.EventHUp is a partial + // FIXME(b/119878986): This waiter.EventHUp is a partial // measure, need to figure out how to translate linux events to // internal events. s.EventRegister(&e, waiter.EventIn|waiter.EventHUp) @@ -370,7 +370,7 @@ func (s *socketOperations) Shutdown(t *kernel.Task, how int) *syserr.Error { // We save the shutdown state because of strange differences on linux // related to recvs on blocking vs. non-blocking sockets after a SHUT_RD. // We need to emulate that behavior on the blocking side. - // TODO: There is a possible race that can exist with loopback, + // TODO(b/120096741): There is a possible race that can exist with loopback, // where data could possibly be lost. s.setShutdownFlags(how) @@ -771,7 +771,7 @@ func (s *socketOperations) SendMsg(t *kernel.Task, src usermem.IOSequence, to [] return 0, syserr.FromError(err) } - // TODO: this needs to change to map directly to a SendMsg syscall + // TODO(bgeffon): this needs to change to map directly to a SendMsg syscall // in the RPC. totalWritten := 0 n, err := rpcSendMsg(t, &pb.SyscallRequest_Sendmsg{&pb.SendmsgRequest{ diff --git a/pkg/sentry/socket/rpcinet/syscall_rpc.proto b/pkg/sentry/socket/rpcinet/syscall_rpc.proto index c056e4c9d..9586f5923 100644 --- a/pkg/sentry/socket/rpcinet/syscall_rpc.proto +++ b/pkg/sentry/socket/rpcinet/syscall_rpc.proto @@ -3,7 +3,7 @@ syntax = "proto3"; // package syscall_rpc is a set of networking related system calls that can be // forwarded to a socket gofer. // -// TODO: Document individual RPCs. +// TODO(b/77963526): Document individual RPCs. package syscall_rpc; message SendmsgRequest { diff --git a/pkg/sentry/strace/strace.go b/pkg/sentry/strace/strace.go index a6d870b44..434a200d9 100644 --- a/pkg/sentry/strace/strace.go +++ b/pkg/sentry/strace/strace.go @@ -722,7 +722,7 @@ func (s SyscallMap) Name(sysno uintptr) string { // N.B. This is not in an init function because we can't be sure all syscall // tables are registered with the kernel when init runs. // -// TODO: remove kernel package dependencies from this +// TODO(gvisor.dev/issue/155): remove kernel package dependencies from this // package and have the kernel package self-initialize all syscall tables. func Initialize() { for _, table := range kernel.SyscallTables() { diff --git a/pkg/sentry/syscalls/linux/error.go b/pkg/sentry/syscalls/linux/error.go index 8759e5e32..304a12dde 100644 --- a/pkg/sentry/syscalls/linux/error.go +++ b/pkg/sentry/syscalls/linux/error.go @@ -89,7 +89,7 @@ func handleIOError(t *kernel.Task, partialResult bool, err, intr error, op strin // side is gone. The partial write is returned. EPIPE will be // returned on the next call. // - // TODO: In some cases SIGPIPE should + // TODO(gvisor.dev/issue/161): In some cases SIGPIPE should // also be sent to the application. return nil case syserror.ErrWouldBlock: diff --git a/pkg/sentry/syscalls/linux/linux64.go b/pkg/sentry/syscalls/linux/linux64.go index be793ca11..b9b4ccbd1 100644 --- a/pkg/sentry/syscalls/linux/linux64.go +++ b/pkg/sentry/syscalls/linux/linux64.go @@ -143,10 +143,10 @@ var AMD64 = &kernel.SyscallTable{ 65: Semop, 66: Semctl, 67: Shmdt, - // 68: @Syscall(Msgget), TODO - // 69: @Syscall(Msgsnd), TODO - // 70: @Syscall(Msgrcv), TODO - // 71: @Syscall(Msgctl), TODO + // 68: @Syscall(Msgget), TODO(b/29354921) + // 69: @Syscall(Msgsnd), TODO(b/29354921) + // 70: @Syscall(Msgrcv), TODO(b/29354921) + // 71: @Syscall(Msgctl), TODO(b/29354921) 72: Fcntl, 73: Flock, 74: Fsync, @@ -197,8 +197,8 @@ var AMD64 = &kernel.SyscallTable{ 119: Setresgid, 120: Getresgid, 121: Getpgid, - // 122: @Syscall(Setfsuid), TODO - // 123: @Syscall(Setfsgid), TODO + // 122: @Syscall(Setfsuid), TODO(b/112851702) + // 123: @Syscall(Setfsgid), TODO(b/112851702) 124: Getsid, 125: Capget, 126: Capset, @@ -217,7 +217,7 @@ var AMD64 = &kernel.SyscallTable{ 136: syscalls.ErrorWithEvent(syscall.ENOSYS), 137: Statfs, 138: Fstatfs, - // 139: @Syscall(Sysfs), TODO + // 139: @Syscall(Sysfs), TODO(gvisor.dev/issue/165) 140: Getpriority, 141: Setpriority, // @Syscall(SchedSetparam, returns:EPERM or ENOSYS, note:Returns EPERM if the process does not have cap_sys_nice; ENOSYS otherwise) @@ -291,7 +291,7 @@ var AMD64 = &kernel.SyscallTable{ // @Syscall(Security, note:Not implemented in Linux) 185: syscalls.Error(syscall.ENOSYS), 186: Gettid, - 187: nil, // @Syscall(Readahead), TODO + 187: nil, // @Syscall(Readahead), TODO(b/29351341) // @Syscall(Setxattr, returns:ENOTSUP, note:Requires filesystem support) 188: syscalls.ErrorWithEvent(syscall.ENOTSUP), // @Syscall(Lsetxattr, returns:ENOTSUP, note:Requires filesystem support) @@ -342,7 +342,7 @@ var AMD64 = &kernel.SyscallTable{ 217: Getdents64, 218: SetTidAddress, 219: RestartSyscall, - // 220: @Syscall(Semtimedop), TODO + // 220: @Syscall(Semtimedop), TODO(b/29354920) 221: Fadvise64, 222: TimerCreate, 223: TimerSettime, @@ -360,16 +360,16 @@ var AMD64 = &kernel.SyscallTable{ 235: Utimes, // @Syscall(Vserver, note:Not implemented by Linux) 236: syscalls.Error(syscall.ENOSYS), // Vserver, not implemented by Linux - // @Syscall(Mbind, returns:EPERM or ENOSYS, note:Returns EPERM if the process does not have cap_sys_nice; ENOSYS otherwise), TODO + // @Syscall(Mbind, returns:EPERM or ENOSYS, note:Returns EPERM if the process does not have cap_sys_nice; ENOSYS otherwise), TODO(b/117792295) 237: syscalls.CapError(linux.CAP_SYS_NICE), // may require cap_sys_nice 238: SetMempolicy, 239: GetMempolicy, - // 240: @Syscall(MqOpen), TODO - // 241: @Syscall(MqUnlink), TODO - // 242: @Syscall(MqTimedsend), TODO - // 243: @Syscall(MqTimedreceive), TODO - // 244: @Syscall(MqNotify), TODO - // 245: @Syscall(MqGetsetattr), TODO + // 240: @Syscall(MqOpen), TODO(b/29354921) + // 241: @Syscall(MqUnlink), TODO(b/29354921) + // 242: @Syscall(MqTimedsend), TODO(b/29354921) + // 243: @Syscall(MqTimedreceive), TODO(b/29354921) + // 244: @Syscall(MqNotify), TODO(b/29354921) + // 245: @Syscall(MqGetsetattr), TODO(b/29354921) 246: syscalls.CapError(linux.CAP_SYS_BOOT), // kexec_load, requires cap_sys_boot 247: Waitid, // @Syscall(AddKey, returns:EACCES, note:Not available to user) @@ -407,22 +407,22 @@ var AMD64 = &kernel.SyscallTable{ 273: syscalls.Error(syscall.ENOSYS), // @Syscall(GetRobustList, note:Obsolete) 274: syscalls.Error(syscall.ENOSYS), - // 275: @Syscall(Splice), TODO - // 276: @Syscall(Tee), TODO + // 275: @Syscall(Splice), TODO(b/29354098) + // 276: @Syscall(Tee), TODO(b/29354098) 277: SyncFileRange, - // 278: @Syscall(Vmsplice), TODO + // 278: @Syscall(Vmsplice), TODO(b/29354098) // @Syscall(MovePages, returns:EPERM or ENOSYS, note:Returns EPERM if the process does not have cap_sys_nice; ENOSYS otherwise) 279: syscalls.CapError(linux.CAP_SYS_NICE), // requires cap_sys_nice (mostly) 280: Utimensat, 281: EpollPwait, - // 282: @Syscall(Signalfd), TODO + // 282: @Syscall(Signalfd), TODO(b/19846426) 283: TimerfdCreate, 284: Eventfd, 285: Fallocate, 286: TimerfdSettime, 287: TimerfdGettime, 288: Accept4, - // 289: @Syscall(Signalfd4), TODO + // 289: @Syscall(Signalfd4), TODO(b/19846426) 290: Eventfd2, 291: EpollCreate1, 292: Dup3, @@ -447,17 +447,17 @@ var AMD64 = &kernel.SyscallTable{ 305: syscalls.CapError(linux.CAP_SYS_TIME), // requires cap_sys_time 306: Syncfs, 307: SendMMsg, - // 308: @Syscall(Setns), TODO + // 308: @Syscall(Setns), TODO(b/29354995) 309: Getcpu, - // 310: @Syscall(ProcessVmReadv), TODO may require cap_sys_ptrace - // 311: @Syscall(ProcessVmWritev), TODO may require cap_sys_ptrace + // 310: @Syscall(ProcessVmReadv), TODO(gvisor.dev/issue/158) may require cap_sys_ptrace + // 311: @Syscall(ProcessVmWritev), TODO(gvisor.dev/issue/158) may require cap_sys_ptrace // @Syscall(Kcmp, returns:EPERM or ENOSYS, note:Requires cap_sys_ptrace) 312: syscalls.CapError(linux.CAP_SYS_PTRACE), // @Syscall(FinitModule, returns:EPERM or ENOSYS, note:Returns EPERM if the process does not have cap_sys_module; ENOSYS otherwise) 313: syscalls.CapError(linux.CAP_SYS_MODULE), - // 314: @Syscall(SchedSetattr), TODO, we have no scheduler - // 315: @Syscall(SchedGetattr), TODO, we have no scheduler - // 316: @Syscall(Renameat2), TODO + // 314: @Syscall(SchedSetattr), TODO(b/118902272), we have no scheduler + // 315: @Syscall(SchedGetattr), TODO(b/118902272), we have no scheduler + // 316: @Syscall(Renameat2), TODO(b/118902772) 317: Seccomp, 318: GetRandom, 319: MemfdCreate, @@ -465,9 +465,9 @@ var AMD64 = &kernel.SyscallTable{ 320: syscalls.CapError(linux.CAP_SYS_BOOT), // @Syscall(Bpf, returns:EPERM or ENOSYS, note:Returns EPERM if the process does not have cap_sys_boot; ENOSYS otherwise) 321: syscalls.CapError(linux.CAP_SYS_ADMIN), // requires cap_sys_admin for all commands - // 322: @Syscall(Execveat), TODO - // 323: @Syscall(Userfaultfd), TODO - // 324: @Syscall(Membarrier), TODO + // 322: @Syscall(Execveat), TODO(b/118901836) + // 323: @Syscall(Userfaultfd), TODO(b/118906345) + // 324: @Syscall(Membarrier), TODO(b/118904897) 325: Mlock2, // Syscalls after 325 are "backports" from versions of Linux after 4.4. // 326: @Syscall(CopyFileRange), diff --git a/pkg/sentry/syscalls/linux/sys_aio.go b/pkg/sentry/syscalls/linux/sys_aio.go index 355071131..61c2647bf 100644 --- a/pkg/sentry/syscalls/linux/sys_aio.go +++ b/pkg/sentry/syscalls/linux/sys_aio.go @@ -120,7 +120,7 @@ func IoDestroy(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sys // Does not exist. return 0, nil, syserror.EINVAL } - // FIXME: Linux blocks until all AIO to the destroyed context is + // FIXME(fvoznika): Linux blocks until all AIO to the destroyed context is // done. return 0, nil, nil } diff --git a/pkg/sentry/syscalls/linux/sys_file.go b/pkg/sentry/syscalls/linux/sys_file.go index 50151f7b6..967464c85 100644 --- a/pkg/sentry/syscalls/linux/sys_file.go +++ b/pkg/sentry/syscalls/linux/sys_file.go @@ -259,7 +259,7 @@ func mknodAt(t *kernel.Task, dirFD kdefs.FD, addr usermem.Addr, mode linux.FileM case linux.ModeCharacterDevice: fallthrough case linux.ModeBlockDevice: - // TODO: We don't support creating block or character + // TODO(b/72101894): We don't support creating block or character // devices at the moment. // // When we start supporting block and character devices, we'll @@ -1532,7 +1532,7 @@ func chown(t *kernel.Task, d *fs.Dirent, uid auth.UID, gid auth.GID) error { owner.GID = kgid } - // FIXME: This is racy; the inode's owner may have changed in + // FIXME(b/62949101): This is racy; the inode's owner may have changed in // the meantime. (Linux holds i_mutex while calling // fs/attr.c:notify_change() => inode_operations::setattr => // inode_change_ok().) diff --git a/pkg/sentry/syscalls/linux/sys_mmap.go b/pkg/sentry/syscalls/linux/sys_mmap.go index 8732861e0..805b251b1 100644 --- a/pkg/sentry/syscalls/linux/sys_mmap.go +++ b/pkg/sentry/syscalls/linux/sys_mmap.go @@ -185,7 +185,7 @@ func Madvise(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysca case linux.MADV_MERGEABLE, linux.MADV_UNMERGEABLE: fallthrough case linux.MADV_DONTDUMP, linux.MADV_DODUMP: - // TODO: Core dumping isn't implemented, so these are + // TODO(b/72045799): Core dumping isn't implemented, so these are // no-ops. fallthrough case linux.MADV_NORMAL, linux.MADV_RANDOM, linux.MADV_SEQUENTIAL, linux.MADV_WILLNEED: @@ -223,7 +223,7 @@ func GetMempolicy(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel. nodeFlag := flags&linux.MPOL_F_NODE != 0 addrFlag := flags&linux.MPOL_F_ADDR != 0 - // TODO: Once sysfs is implemented, report a single numa node in + // TODO(rahat): Once sysfs is implemented, report a single numa node in // /sys/devices/system/node. if nodemask != 0 && maxnode < 1 { return 0, nil, syserror.EINVAL diff --git a/pkg/sentry/syscalls/linux/sys_read.go b/pkg/sentry/syscalls/linux/sys_read.go index 8105e9b43..50c7d7a74 100644 --- a/pkg/sentry/syscalls/linux/sys_read.go +++ b/pkg/sentry/syscalls/linux/sys_read.go @@ -192,7 +192,7 @@ func Preadv(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscal } // Preadv2 implements linux syscall preadv2(2). -// TODO: Implement RWF_HIPRI functionality. +// TODO(b/120162627): Implement RWF_HIPRI functionality. func Preadv2(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { // While the syscall is // preadv2(int fd, struct iovec* iov, int iov_cnt, off_t offset, int flags) diff --git a/pkg/sentry/syscalls/linux/sys_socket.go b/pkg/sentry/syscalls/linux/sys_socket.go index 30ccc3f66..c8748958a 100644 --- a/pkg/sentry/syscalls/linux/sys_socket.go +++ b/pkg/sentry/syscalls/linux/sys_socket.go @@ -317,7 +317,7 @@ func accept(t *kernel.Task, fd kdefs.FD, addr usermem.Addr, addrLen usermem.Addr return 0, syserror.ConvertIntr(e.ToError(), kernel.ERESTARTSYS) } if peerRequested { - // NOTE: Linux does not give you an error if it can't + // NOTE(magi): Linux does not give you an error if it can't // write the data back out so neither do we. if err := writeAddress(t, peer, peerLen, addr, addrLen); err == syscall.EINVAL { return 0, err @@ -735,7 +735,7 @@ func recvSingleMsg(t *kernel.Task, s socket.Socket, msgPtr usermem.Addr, flags i return 0, err } - // FIXME: Pretend we have an empty error queue. + // FIXME(b/63594852): Pretend we have an empty error queue. if flags&linux.MSG_ERRQUEUE != 0 { return 0, syscall.EAGAIN } diff --git a/pkg/sentry/syscalls/linux/sys_thread.go b/pkg/sentry/syscalls/linux/sys_thread.go index 61cafefb9..ddcb5b789 100644 --- a/pkg/sentry/syscalls/linux/sys_thread.go +++ b/pkg/sentry/syscalls/linux/sys_thread.go @@ -350,7 +350,7 @@ func Waitid(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscal } si.SetPid(int32(wr.TID)) si.SetUid(int32(wr.UID)) - // TODO: convert kernel.ExitStatus to functions and make + // TODO(b/73541790): convert kernel.ExitStatus to functions and make // WaitResult.Status a linux.WaitStatus s := syscall.WaitStatus(wr.Status) switch { diff --git a/pkg/sentry/syscalls/linux/sys_write.go b/pkg/sentry/syscalls/linux/sys_write.go index a5ad7efb2..e405608c4 100644 --- a/pkg/sentry/syscalls/linux/sys_write.go +++ b/pkg/sentry/syscalls/linux/sys_write.go @@ -192,8 +192,8 @@ func Pwritev(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sysca } // Pwritev2 implements linux syscall pwritev2(2). -// TODO: Implement RWF_HIPRI functionality. -// TODO: Implement O_SYNC and D_SYNC functionality. +// TODO(b/120162627): Implement RWF_HIPRI functionality. +// TODO(b/120161091): Implement O_SYNC and D_SYNC functionality. func Pwritev2(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.SyscallControl, error) { // While the syscall is // pwritev2(int fd, struct iovec* iov, int iov_cnt, off_t offset, int flags) diff --git a/pkg/sentry/time/calibrated_clock.go b/pkg/sentry/time/calibrated_clock.go index c8cf4eca4..a98bcd7de 100644 --- a/pkg/sentry/time/calibrated_clock.go +++ b/pkg/sentry/time/calibrated_clock.go @@ -37,7 +37,7 @@ var fallbackMetric = metric.MustCreateNewUint64Metric("/time/fallback", false /* // clock. type CalibratedClock struct { // mu protects the fields below. - // TODO: consider a sequence counter for read locking. + // TODO(mpratt): consider a sequence counter for read locking. mu sync.RWMutex // ref sample the reference clock that this clock is calibrated @@ -140,7 +140,7 @@ func (c *CalibratedClock) updateParams(actual Parameters) { // N.B. logErrorAdjustment will have already logged the error // at warning level. // - // TODO: We could allow Realtime clock jumps here. + // TODO(mpratt): We could allow Realtime clock jumps here. c.resetLocked("Extreme clock error.") return } @@ -229,7 +229,7 @@ func (c *CalibratedClock) GetTime() (int64, error) { // CalibratedClocks contains calibrated monotonic and realtime clocks. // -// TODO: We know that Linux runs the monotonic and realtime clocks at +// TODO(mpratt): We know that Linux runs the monotonic and realtime clocks at // the same rate, so rather than tracking both individually, we could do one // calibration for both clocks. type CalibratedClocks struct { diff --git a/pkg/sentry/time/parameters.go b/pkg/sentry/time/parameters.go index f3ad58454..8568b1193 100644 --- a/pkg/sentry/time/parameters.go +++ b/pkg/sentry/time/parameters.go @@ -43,7 +43,7 @@ const ( // These statements assume that the host clock does not change. Actual // error will depend upon host clock changes. // - // TODO: make error correction more robust to delayed + // TODO(b/68779214): make error correction more robust to delayed // updates. ApproxUpdateInterval = 1 * time.Second diff --git a/pkg/sentry/usermem/usermem.go b/pkg/sentry/usermem/usermem.go index 99766a803..4c7d5014a 100644 --- a/pkg/sentry/usermem/usermem.go +++ b/pkg/sentry/usermem/usermem.go @@ -28,7 +28,7 @@ import ( // IO provides access to the contents of a virtual memory space. // -// FIXME: Implementations of IO cannot expect ctx to contain any +// FIXME(b/38173783): Implementations of IO cannot expect ctx to contain any // meaningful data. type IO interface { // CopyOut copies len(src) bytes from src to the memory mapped at addr. It @@ -85,7 +85,7 @@ type IO interface { // order. CopyInTo(ctx context.Context, ars AddrRangeSeq, dst safemem.Writer, opts IOOpts) (int64, error) - // TODO: The requirement that CopyOutFrom/CopyInTo call src/dst + // TODO(jamieliu): The requirement that CopyOutFrom/CopyInTo call src/dst // at most once, which is unnecessary in most cases, forces implementations // to gather safemem.Blocks into a single slice to pass to src/dst. Add // CopyOutFromIter/CopyInToIter, which relaxes this restriction, to avoid diff --git a/pkg/sentry/watchdog/watchdog.go b/pkg/sentry/watchdog/watchdog.go index c49b537a5..b4f1e3a4f 100644 --- a/pkg/sentry/watchdog/watchdog.go +++ b/pkg/sentry/watchdog/watchdog.go @@ -236,7 +236,7 @@ func (w *Watchdog) runTurn() { if !ok { // New stuck task detected. // - // TODO: Tasks blocked doing IO may be considered stuck in kernel. + // TODO(b/65849403): Tasks blocked doing IO may be considered stuck in kernel. tc = &offender{lastUpdateTime: lastUpdateTime} stuckTasks.Increment() newTaskFound = true diff --git a/pkg/syserr/syserr.go b/pkg/syserr/syserr.go index dad83e80c..232634dd4 100644 --- a/pkg/syserr/syserr.go +++ b/pkg/syserr/syserr.go @@ -49,7 +49,7 @@ func New(message string, linuxTranslation *linux.Errno) *Error { return err } - // TODO: Remove this. + // TODO(b/34162363): Remove this. errno := linuxTranslation.Number() if errno <= 0 || errno >= len(linuxBackwardsTranslations) { panic(fmt.Sprint("invalid errno: ", errno)) @@ -106,12 +106,12 @@ type linuxBackwardsTranslation struct { ok bool } -// TODO: Remove this. +// TODO(b/34162363): Remove this. var linuxBackwardsTranslations [maxErrno]linuxBackwardsTranslation // ToError translates an Error to a corresponding error value. // -// TODO: Remove this. +// TODO(b/34162363): Remove this. func (e *Error) ToError() error { if e == nil { return nil @@ -138,7 +138,7 @@ func (e *Error) ToLinux() *linux.Errno { return e.errno } -// TODO: Remove or replace most of these errors. +// TODO(b/34162363): Remove or replace most of these errors. // // Some of the errors should be replaced with package specific errors and // others should be removed entirely. @@ -278,7 +278,7 @@ var ( // FromError converts a generic error to an *Error. // -// TODO: Remove this function. +// TODO(b/34162363): Remove this function. func FromError(err error) *Error { if err == nil { return nil diff --git a/pkg/tcpip/network/ipv4/icmp.go b/pkg/tcpip/network/ipv4/icmp.go index ed9a4eee5..1c3acda4b 100644 --- a/pkg/tcpip/network/ipv4/icmp.go +++ b/pkg/tcpip/network/ipv4/icmp.go @@ -64,7 +64,7 @@ func (e *endpoint) handleICMP(r *stack.Route, netHeader buffer.View, vv buffer.V } h := header.ICMPv4(v) - // TODO: Meaningfully handle all ICMP types. + // TODO(b/112892170): Meaningfully handle all ICMP types. switch h.Type() { case header.ICMPv4Echo: received.Echo.Increment() diff --git a/pkg/tcpip/network/ipv6/icmp.go b/pkg/tcpip/network/ipv6/icmp.go index 3210e6fc7..be28be36d 100644 --- a/pkg/tcpip/network/ipv6/icmp.go +++ b/pkg/tcpip/network/ipv6/icmp.go @@ -73,7 +73,7 @@ func (e *endpoint) handleICMP(r *stack.Route, netHeader buffer.View, vv buffer.V } h := header.ICMPv6(v) - // TODO: Meaningfully handle all ICMP types. + // TODO(b/112892170): Meaningfully handle all ICMP types. switch h.Type() { case header.ICMPv6PacketTooBig: received.PacketTooBig.Increment() @@ -247,7 +247,7 @@ func (*protocol) LinkAddressRequest(addr, localAddr tcpip.Address, linkEP stack. DstAddr: r.RemoteAddress, }) - // TODO: count this in ICMP stats. + // TODO(stijlist): count this in ICMP stats. return linkEP.WritePacket(r, nil /* gso */, hdr, buffer.VectorisedView{}, ProtocolNumber) } diff --git a/pkg/tcpip/stack/nic.go b/pkg/tcpip/stack/nic.go index 8b6c17a90..c18571b0f 100644 --- a/pkg/tcpip/stack/nic.go +++ b/pkg/tcpip/stack/nic.go @@ -176,7 +176,7 @@ func (n *NIC) primaryEndpoint(protocol tcpip.NetworkProtocolNumber) *referencedN for e := list.Front(); e != nil; e = e.Next() { r := e.(*referencedNetworkEndpoint) - // TODO: allow broadcast address when SO_BROADCAST is set. + // TODO(crawshaw): allow broadcast address when SO_BROADCAST is set. switch r.ep.ID().LocalAddress { case header.IPv4Broadcast, header.IPv4Any: continue @@ -476,7 +476,7 @@ func (n *NIC) DeliverNetworkPacket(linkEP LinkEndpoint, remote, _ tcpip.LinkAddr n.mu.RUnlock() if ok && ref.tryIncRef() { r.RemoteAddress = src - // TODO: Update the source NIC as well. + // TODO(b/123449044): Update the source NIC as well. ref.ep.HandlePacket(&r, vv) ref.decRef() } else { @@ -485,7 +485,7 @@ func (n *NIC) DeliverNetworkPacket(linkEP LinkEndpoint, remote, _ tcpip.LinkAddr hdr := buffer.NewPrependableFromView(vv.First()) vv.RemoveFirst() - // TODO: use route.WritePacket. + // TODO(b/128629022): use route.WritePacket. if err := n.linkEP.WritePacket(&r, nil /* gso */, hdr, vv, protocol); err != nil { r.Stats().IP.OutgoingPacketErrors.Increment() } else { diff --git a/pkg/tcpip/stack/stack.go b/pkg/tcpip/stack/stack.go index 8f7b6f781..cb9ffe9c2 100644 --- a/pkg/tcpip/stack/stack.go +++ b/pkg/tcpip/stack/stack.go @@ -476,7 +476,7 @@ func (s *Stack) Stats() tcpip.Stats { // SetForwarding enables or disables the packet forwarding between NICs. func (s *Stack) SetForwarding(enable bool) { - // TODO: Expose via /proc/sys/net/ipv4/ip_forward. + // TODO(igudger, bgeffon): Expose via /proc/sys/net/ipv4/ip_forward. s.mu.Lock() s.forwarding = enable s.mu.Unlock() @@ -484,7 +484,7 @@ func (s *Stack) SetForwarding(enable bool) { // Forwarding returns if the packet forwarding between NICs is enabled. func (s *Stack) Forwarding() bool { - // TODO: Expose via /proc/sys/net/ipv4/ip_forward. + // TODO(igudger, bgeffon): Expose via /proc/sys/net/ipv4/ip_forward. s.mu.RLock() defer s.mu.RUnlock() return s.forwarding diff --git a/pkg/tcpip/stack/stack_global_state.go b/pkg/tcpip/stack/stack_global_state.go index f2c6c9a8d..3d7e4b719 100644 --- a/pkg/tcpip/stack/stack_global_state.go +++ b/pkg/tcpip/stack/stack_global_state.go @@ -15,5 +15,5 @@ package stack // StackFromEnv is the global stack created in restore run. -// FIXME +// FIXME(b/36201077) var StackFromEnv *Stack diff --git a/pkg/tcpip/stack/transport_test.go b/pkg/tcpip/stack/transport_test.go index 0c2589083..2df974bf2 100644 --- a/pkg/tcpip/stack/transport_test.go +++ b/pkg/tcpip/stack/transport_test.go @@ -453,7 +453,7 @@ func TestTransportForwarding(t *testing.T) { s := stack.New([]string{"fakeNet"}, []string{"fakeTrans"}, stack.Options{}) s.SetForwarding(true) - // TODO: Change this to a channel NIC. + // TODO(b/123449044): Change this to a channel NIC. id1 := loopback.New() if err := s.CreateNIC(1, id1); err != nil { t.Fatalf("CreateNIC #1 failed: %v", err) diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index 80cd6b4e5..b09137f08 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -444,7 +444,7 @@ type PasscredOption int // TCPInfoOption is used by GetSockOpt to expose TCP statistics. // -// TODO: Add and populate stat fields. +// TODO(b/64800844): Add and populate stat fields. type TCPInfoOption struct { RTT time.Duration RTTVar time.Duration diff --git a/pkg/tcpip/transport/raw/raw.go b/pkg/tcpip/transport/raw/raw.go index 8dada2e4f..f0f60ce91 100644 --- a/pkg/tcpip/transport/raw/raw.go +++ b/pkg/tcpip/transport/raw/raw.go @@ -100,7 +100,7 @@ type endpoint struct { } // NewEndpoint returns a raw endpoint for the given protocols. -// TODO: IP_HDRINCL, IPPROTO_RAW, and AF_PACKET. +// TODO(b/129292371): IP_HDRINCL, IPPROTO_RAW, and AF_PACKET. func NewEndpoint(stack *stack.Stack, netProto tcpip.NetworkProtocolNumber, transProto tcpip.TransportProtocolNumber, waiterQueue *waiter.Queue) (tcpip.Endpoint, *tcpip.Error) { if netProto != header.IPv4ProtocolNumber { return nil, tcpip.ErrUnknownProtocol diff --git a/pkg/tcpip/transport/tcp/BUILD b/pkg/tcpip/transport/tcp/BUILD index e5c05f8c0..d44d63e95 100644 --- a/pkg/tcpip/transport/tcp/BUILD +++ b/pkg/tcpip/transport/tcp/BUILD @@ -73,7 +73,7 @@ go_test( "tcp_test.go", "tcp_timestamp_test.go", ], - # FIXME + # FIXME(b/68809571) tags = ["flaky"], deps = [ ":tcp", diff --git a/pkg/unet/unet.go b/pkg/unet/unet.go index deeea078d..114fb8c5b 100644 --- a/pkg/unet/unet.go +++ b/pkg/unet/unet.go @@ -211,7 +211,7 @@ func SocketPair(packet bool) (*Socket, *Socket, error) { // variable between our two sockets. We only use SocketPair in tests // anyway. // - // NOTE: This is purely due to the fact that the raw + // NOTE(b/27107811): This is purely due to the fact that the raw // syscall does not serve as a boundary for the sanitizer. var race int32 a, err := NewSocket(fds[0]) diff --git a/pkg/unet/unet_test.go b/pkg/unet/unet_test.go index ecc670925..db5485539 100644 --- a/pkg/unet/unet_test.go +++ b/pkg/unet/unet_test.go @@ -40,7 +40,7 @@ func randomFilename() (string, error) { return "", err } - // NOTE: We try to use relative path if possible. This is + // NOTE(b/26918832): We try to use relative path if possible. This is // to help conforming to the unix path length limit. if rel, err := filepath.Rel(cwd, file); err == nil { return rel, nil |