From f4ce43e1f426148d99c28c1b0e5c43ddda17a8cb Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Mon, 29 Apr 2019 14:03:04 -0700 Subject: [PATCH] Allow and document bug ids in gVisor codebase. PiperOrigin-RevId: 245818639 Change-Id: I03703ef0fb9b6675955637b9fe2776204c545789 --- CONTRIBUTING.md | 7 +++ pkg/cpuid/cpuid_test.go | 2 +- pkg/dhcp/client.go | 2 +- pkg/log/glog.go | 2 +- pkg/metric/metric.go | 4 +- pkg/segment/set.go | 2 +- pkg/segment/test/set_functions.go | 2 +- pkg/sentry/arch/arch.go | 2 +- pkg/sentry/arch/arch_amd64.go | 4 +- pkg/sentry/arch/arch_x86.go | 2 +- pkg/sentry/arch/signal_amd64.go | 6 +- pkg/sentry/arch/stack.go | 6 +- pkg/sentry/context/context.go | 2 +- pkg/sentry/control/proc.go | 2 +- pkg/sentry/fs/README.md | 2 +- pkg/sentry/fs/ashmem/area.go | 4 +- pkg/sentry/fs/binder/binder.go | 22 +++---- pkg/sentry/fs/dentry.go | 2 +- pkg/sentry/fs/dirent.go | 8 +-- pkg/sentry/fs/file.go | 2 +- pkg/sentry/fs/file_overlay.go | 4 +- pkg/sentry/fs/fsutil/file.go | 8 +-- pkg/sentry/fs/fsutil/inode_cached.go | 4 +- pkg/sentry/fs/gofer/cache_policy.go | 4 +- pkg/sentry/fs/gofer/file.go | 2 +- pkg/sentry/fs/gofer/file_state.go | 2 +- pkg/sentry/fs/gofer/handles.go | 2 +- pkg/sentry/fs/gofer/inode.go | 6 +- pkg/sentry/fs/gofer/inode_state.go | 2 +- pkg/sentry/fs/gofer/session.go | 2 +- pkg/sentry/fs/gofer/session_state.go | 2 +- pkg/sentry/fs/host/fs.go | 4 +- pkg/sentry/fs/host/inode.go | 10 ++-- pkg/sentry/fs/inode.go | 6 +- pkg/sentry/fs/inode_operations.go | 2 +- pkg/sentry/fs/inode_overlay.go | 6 +- pkg/sentry/fs/mount.go | 4 +- pkg/sentry/fs/mount_test.go | 2 +- pkg/sentry/fs/proc/README.md | 12 ++-- pkg/sentry/fs/proc/fds.go | 2 +- pkg/sentry/fs/proc/loadavg.go | 2 +- pkg/sentry/fs/proc/meminfo.go | 6 +- pkg/sentry/fs/proc/mounts.go | 2 +- pkg/sentry/fs/proc/net.go | 2 +- pkg/sentry/fs/proc/stat.go | 12 ++-- pkg/sentry/fs/proc/sys_net.go | 2 +- pkg/sentry/fs/proc/task.go | 8 +-- pkg/sentry/fs/proc/version.go | 2 +- pkg/sentry/fs/ramfs/dir.go | 2 +- pkg/sentry/fs/tmpfs/fs.go | 2 +- pkg/sentry/fs/tmpfs/inode_file.go | 2 +- pkg/sentry/fs/tmpfs/tmpfs.go | 2 +- pkg/sentry/fs/tty/dir.go | 6 +- pkg/sentry/fs/tty/fs.go | 2 +- pkg/sentry/fs/tty/master.go | 6 +- pkg/sentry/fs/tty/slave.go | 6 +- pkg/sentry/kernel/auth/credentials.go | 2 +- pkg/sentry/kernel/auth/user_namespace.go | 2 +- pkg/sentry/kernel/pending_signals.go | 2 +- pkg/sentry/kernel/ptrace.go | 4 +- pkg/sentry/kernel/rseq.go | 2 +- pkg/sentry/kernel/sched/cpuset.go | 2 +- pkg/sentry/kernel/semaphore/semaphore.go | 6 +- pkg/sentry/kernel/shm/shm.go | 2 +- pkg/sentry/kernel/syscalls.go | 2 +- pkg/sentry/kernel/task_context.go | 2 +- pkg/sentry/kernel/task_exec.go | 2 +- pkg/sentry/kernel/task_exit.go | 4 +- pkg/sentry/kernel/task_identity.go | 2 +- pkg/sentry/kernel/task_run.go | 2 +- pkg/sentry/kernel/task_signals.go | 4 +- pkg/sentry/kernel/task_stop.go | 2 +- pkg/sentry/loader/loader.go | 2 +- pkg/sentry/loader/vdso.go | 6 +- pkg/sentry/memmap/memmap.go | 2 +- pkg/sentry/mm/aio_context.go | 2 +- pkg/sentry/mm/procfs.go | 10 ++-- pkg/sentry/mm/special_mappable.go | 2 +- pkg/sentry/mm/syscalls.go | 6 +- pkg/sentry/mm/vma.go | 2 +- pkg/sentry/platform/kvm/kvm_amd64_unsafe.go | 2 +- pkg/sentry/platform/platform.go | 2 +- pkg/sentry/platform/ptrace/subprocess.go | 2 +- pkg/sentry/platform/ring0/x86.go | 4 +- pkg/sentry/sighandling/sighandling.go | 2 +- pkg/sentry/sighandling/sighandling_unsafe.go | 2 +- pkg/sentry/socket/epsocket/epsocket.go | 32 +++++----- pkg/sentry/socket/epsocket/save_restore.go | 2 +- pkg/sentry/socket/epsocket/stack.go | 2 +- pkg/sentry/socket/hostinet/socket.go | 2 +- pkg/sentry/socket/netlink/route/protocol.go | 8 +-- pkg/sentry/socket/netlink/socket.go | 10 ++-- pkg/sentry/socket/rpcinet/conn/conn.go | 2 +- .../socket/rpcinet/notifier/notifier.go | 4 +- pkg/sentry/socket/rpcinet/socket.go | 6 +- pkg/sentry/socket/rpcinet/syscall_rpc.proto | 2 +- pkg/sentry/strace/strace.go | 2 +- pkg/sentry/syscalls/linux/error.go | 2 +- pkg/sentry/syscalls/linux/linux64.go | 60 +++++++++---------- pkg/sentry/syscalls/linux/sys_aio.go | 2 +- pkg/sentry/syscalls/linux/sys_file.go | 4 +- pkg/sentry/syscalls/linux/sys_mmap.go | 4 +- pkg/sentry/syscalls/linux/sys_read.go | 2 +- pkg/sentry/syscalls/linux/sys_socket.go | 4 +- pkg/sentry/syscalls/linux/sys_thread.go | 2 +- pkg/sentry/syscalls/linux/sys_write.go | 4 +- pkg/sentry/time/calibrated_clock.go | 6 +- pkg/sentry/time/parameters.go | 2 +- pkg/sentry/usermem/usermem.go | 4 +- pkg/sentry/watchdog/watchdog.go | 2 +- pkg/syserr/syserr.go | 10 ++-- pkg/tcpip/network/ipv4/icmp.go | 2 +- pkg/tcpip/network/ipv6/icmp.go | 4 +- pkg/tcpip/stack/nic.go | 6 +- pkg/tcpip/stack/stack.go | 4 +- pkg/tcpip/stack/stack_global_state.go | 2 +- pkg/tcpip/stack/transport_test.go | 2 +- pkg/tcpip/tcpip.go | 2 +- pkg/tcpip/transport/raw/raw.go | 2 +- pkg/tcpip/transport/tcp/BUILD | 2 +- pkg/unet/unet.go | 2 +- pkg/unet/unet_test.go | 2 +- runsc/boot/controller.go | 4 +- runsc/boot/fs.go | 6 +- runsc/boot/loader.go | 2 +- runsc/cmd/checkpoint.go | 2 +- runsc/container/container.go | 2 +- runsc/container/container_test.go | 4 +- runsc/sandbox/sandbox.go | 6 +- runsc/specutils/specutils.go | 4 +- test/syscalls/BUILD | 6 +- test/syscalls/build_defs.bzl | 4 +- test/syscalls/linux/32bit.cc | 14 ++--- test/syscalls/linux/aio.cc | 2 +- test/syscalls/linux/chmod.cc | 2 +- test/syscalls/linux/epoll.cc | 2 +- test/syscalls/linux/exec_binary.cc | 12 ++-- test/syscalls/linux/file_base.h | 4 +- test/syscalls/linux/ioctl.cc | 4 +- test/syscalls/linux/ip_socket_test_util.cc | 2 +- test/syscalls/linux/lseek.cc | 2 +- test/syscalls/linux/mkdir.cc | 2 +- test/syscalls/linux/mmap.cc | 18 +++--- test/syscalls/linux/open.cc | 2 +- test/syscalls/linux/partial_bad_buffer.cc | 18 +++--- test/syscalls/linux/pipe.cc | 6 +- test/syscalls/linux/proc.cc | 32 +++++----- test/syscalls/linux/proc_pid_smaps.cc | 2 +- test/syscalls/linux/ptrace.cc | 2 +- test/syscalls/linux/pwrite64.cc | 2 +- test/syscalls/linux/readv_socket.cc | 2 +- test/syscalls/linux/rtsignal.cc | 2 +- test/syscalls/linux/socket_inet_loopback.cc | 10 ++-- ...et_ipv4_udp_unbound_external_networking.cc | 4 +- test/syscalls/linux/socket_netlink_route.cc | 4 +- test/syscalls/linux/socket_stream_blocking.cc | 2 +- test/syscalls/linux/socket_test_util.cc | 2 +- test/syscalls/linux/socket_unix.cc | 16 ++--- test/syscalls/linux/socket_unix_dgram.cc | 2 +- .../linux/socket_unix_dgram_non_blocking.cc | 2 +- test/syscalls/linux/socket_unix_non_stream.cc | 10 ++-- .../linux/socket_unix_unbound_seqpacket.cc | 2 +- .../linux/socket_unix_unbound_stream.cc | 4 +- test/syscalls/linux/stat.cc | 2 +- test/syscalls/linux/stat_times.cc | 8 +-- test/syscalls/linux/tcp_socket.cc | 2 +- test/syscalls/linux/tkill.cc | 2 +- test/syscalls/linux/udp_bind.cc | 4 +- test/syscalls/linux/uidgid.cc | 2 +- test/syscalls/linux/utimes.cc | 4 +- test/syscalls/linux/wait.cc | 2 +- test/syscalls/linux/write.cc | 2 +- .../gvsync/downgradable_rwmutex_unsafe.go | 2 +- vdso/cycle_clock.h | 2 +- vdso/vdso_amd64.lds | 2 +- vdso/vdso_arm64.lds | 2 +- 176 files changed, 403 insertions(+), 396 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d6dafc595..238dd6665 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -126,6 +126,13 @@ change. When approved, the change will be submitted by a team member and automatically merged into the repository. +### Bug IDs + +Some TODOs and NOTEs sprinkled throughout the code have associated IDs of the +form b/1234. These correspond to bugs in our internal bug tracker. Eventually +these bugs will be moved to the GitHub Issues, but until then they can simply be +ignored. + ### The small print Contributions made by corporations are covered by a different agreement than the 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 diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 2488981f9..712c50ee9 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -231,7 +231,7 @@ func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { } // Prevent CIDs containing ".." from confusing the sentry when creating // /containers/ directory. - // TODO: Once we have multiple independent roots, this + // TODO(b/129293409): Once we have multiple independent roots, this // check won't be necessary. if path.Clean(args.CID) != args.CID { return fmt.Errorf("container ID shouldn't contain directory traversals such as \"..\": %q", args.CID) @@ -352,7 +352,7 @@ func (cm *containerManager) Restore(o *RestoreOpts, _ *struct{}) error { return fmt.Errorf("creating network: %v", err) } if eps, ok := networkStack.(*epsocket.Stack); ok { - stack.StackFromEnv = eps.Stack // FIXME + stack.StackFromEnv = eps.Stack // FIXME(b/36201077) } info, err := o.FilePayload.Files[0].Stat() if err != nil { diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 761142d98..07061b9b3 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -274,7 +274,7 @@ func getMountNameAndOptions(conf *Config, m specs.Mount, fds *fdDispenser) (stri useOverlay = conf.Overlay && !mountFlags(m.Options).ReadOnly default: - // TODO: Support all the mount types and make this a + // TODO(nlacasse): Support all the mount types and make this a // fatal error. Most applications will "just work" without // them, so this is a warning for now. // we do not support. @@ -425,7 +425,7 @@ func addRestoreMount(conf *Config, renv *fs.RestoreEnvironment, m specs.Mount, f if err != nil { return err } - // TODO: Fix this when we support all the mount types and + // TODO(nlacasse): Fix this when we support all the mount types and // make this a fatal error. if fsName == "" { return nil @@ -475,7 +475,7 @@ func createRestoreEnvironment(spec *specs.Spec, conf *Config, fds *fdDispenser) } } - // TODO: handle '/tmp' properly (see mountTmp()). + // TODO(b/67958150): handle '/tmp' properly (see mountTmp()). if !tmpMounted { tmpMount := specs.Mount{ Type: tmpfs, diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 48ecb2626..75ec19c32 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -577,7 +577,7 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config // sentry currently supports only 1 mount namespace, which is tied to a // single user namespace. Thus we must run in the same user namespace // to access mounts. - // TODO: Create a new mount namespace for the container. + // TODO(b/63601033): Create a new mount namespace for the container. creds := auth.NewUserCredentials( auth.KUID(spec.Process.User.UID), auth.KGID(spec.Process.User.GID), diff --git a/runsc/cmd/checkpoint.go b/runsc/cmd/checkpoint.go index d8f748aa0..f722df055 100644 --- a/runsc/cmd/checkpoint.go +++ b/runsc/cmd/checkpoint.go @@ -105,7 +105,7 @@ func (c *Checkpoint) Execute(_ context.Context, f *flag.FlagSet, args ...interfa return subcommands.ExitSuccess } - // TODO: Make it possible to restore into same container. + // TODO(b/110843694): Make it possible to restore into same container. // For now, we can fake it by destroying the container and making a // new container with the same ID. This hack does not work with docker // which uses the container pid to ensure that the restore-container is diff --git a/runsc/container/container.go b/runsc/container/container.go index 1bed1a97e..a30c217f7 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -529,7 +529,7 @@ func (c *Container) WaitPID(pid int32, clearStatus bool) (syscall.WaitStatus, er // SignalContainer sends the signal to the container. If all is true and signal // is SIGKILL, then waits for all processes to exit before returning. // SignalContainer returns an error if the container is already stopped. -// TODO: Distinguish different error types. +// TODO(b/113680494): Distinguish different error types. func (c *Container) SignalContainer(sig syscall.Signal, all bool) error { log.Debugf("Signal container %q: %v", c.ID, sig) // Signaling container in Stopped state is allowed. When all=false, diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index 9fe584aa3..603c4d929 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -242,10 +242,10 @@ func configs(opts ...configOption) []*boot.Config { case overlay: c.Overlay = true case kvm: - // TODO: KVM tests are flaky. Disable until fixed. + // TODO(b/112165693): KVM tests are flaky. Disable until fixed. continue - // TODO: KVM doesn't work with --race. + // TODO(b/68787993): KVM doesn't work with --race. if testutil.RaceEnabled { continue } diff --git a/runsc/sandbox/sandbox.go b/runsc/sandbox/sandbox.go index 92495c69e..48a0dafe2 100644 --- a/runsc/sandbox/sandbox.go +++ b/runsc/sandbox/sandbox.go @@ -267,7 +267,7 @@ func (s *Sandbox) Event(cid string) (*boot.Event, error) { defer conn.Close() var e boot.Event - // TODO: Pass in the container id (cid) here. The sandbox + // TODO(b/129292330): Pass in the container id (cid) here. The sandbox // should return events only for that container. if err := conn.Call(boot.ContainerEvent, nil, &e); err != nil { return nil, fmt.Errorf("retrieving event data from sandbox: %v", err) @@ -457,7 +457,7 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund } if conf.Platform == boot.PlatformPtrace { - // TODO: Also set a new PID namespace so that we limit + // TODO(b/75837838): Also set a new PID namespace so that we limit // access to other host processes. log.Infof("Sandbox will be started in the current PID namespace") } else { @@ -520,7 +520,7 @@ func (s *Sandbox) createSandboxProcess(spec *specs.Spec, conf *boot.Config, bund // root for itself, so it has to have the CAP_SYS_ADMIN // capability. // - // FIXME: The current implementations of + // FIXME(b/122554829): The current implementations of // os/exec doesn't allow to set ambient capabilities if // a process is started in a new user namespace. As a // workaround, we start the sandbox process with the 0 diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index 32f81b8d4..ac85bec71 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -90,7 +90,7 @@ func ValidateSpec(spec *specs.Spec) error { log.Warningf("AppArmor profile %q is being ignored", spec.Process.ApparmorProfile) } - // TODO: Apply seccomp to application inside sandbox. + // TODO(b/72226747): Apply seccomp to application inside sandbox. if spec.Linux != nil && spec.Linux.Seccomp != nil { log.Warningf("Seccomp spec is being ignored") } @@ -220,7 +220,7 @@ func Capabilities(enableRaw bool, specCaps *specs.LinuxCapabilities) (*auth.Task if caps.PermittedCaps, err = capsFromNames(specCaps.Permitted, skipSet); err != nil { return nil, err } - // TODO: Support ambient capabilities. + // TODO(nlacasse): Support ambient capabilities. } return &caps, nil } diff --git a/test/syscalls/BUILD b/test/syscalls/BUILD index 94e0f24e0..d35f59433 100644 --- a/test/syscalls/BUILD +++ b/test/syscalls/BUILD @@ -277,7 +277,7 @@ syscall_test(test = "//test/syscalls/linux:sendfile_test") syscall_test(test = "//test/syscalls/linux:sigaction_test") -# TODO: Enable once the test passes in runsc. +# TODO(b/119826902): Enable once the test passes in runsc. # syscall_test(test = "//test/syscalls/linux:sigaltstack_test") syscall_test(test = "//test/syscalls/linux:sigiret_test") @@ -414,7 +414,7 @@ syscall_test( ) syscall_test( - # NOTE: Large sendmsg may stall a long time. + # NOTE(b/116636318): Large sendmsg may stall a long time. size = "enormous", test = "//test/syscalls/linux:socket_unix_dgram_local_test", ) @@ -437,7 +437,7 @@ syscall_test( ) syscall_test( - # NOTE: Large sendmsg may stall a long time. + # NOTE(b/116636318): Large sendmsg may stall a long time. size = "enormous", test = "//test/syscalls/linux:socket_unix_seqpacket_local_test", ) diff --git a/test/syscalls/build_defs.bzl b/test/syscalls/build_defs.bzl index 610b030b2..cd74a769d 100644 --- a/test/syscalls/build_defs.bzl +++ b/test/syscalls/build_defs.bzl @@ -78,10 +78,10 @@ def _syscall_test( tags += [full_platform, "file_" + file_access] # Add tag to prevent the tests from running in a Bazel sandbox. - # TODO: Make the tests run without this tag. + # TODO(b/120560048): Make the tests run without this tag. tags.append("no-sandbox") - # TODO: KVM tests are tagged "manual" to until the platform is + # TODO(b/112165693): KVM tests are tagged "manual" to until the platform is # more stable. if platform == "kvm": tags += ["manual"] diff --git a/test/syscalls/linux/32bit.cc b/test/syscalls/linux/32bit.cc index 230648c9b..78baf548e 100644 --- a/test/syscalls/linux/32bit.cc +++ b/test/syscalls/linux/32bit.cc @@ -80,11 +80,11 @@ constexpr int kExitCode = 42; TEST(Syscall32Bit, Int80) { switch (GvisorPlatform()) { case Platform::kKVM: - // TODO: 32-bit segments are broken (but not explictly + // TODO(b/111805002): 32-bit segments are broken (but not explictly // disabled). return; case Platform::kPtrace: - // TODO: The ptrace platform does not have a + // TODO(gvisor.dev/issue/167): The ptrace platform does not have a // consistent story here. return; case Platform::kNative: @@ -99,10 +99,10 @@ TEST(Syscall32Bit, Int80) { TEST(Syscall32Bit, Sysenter) { switch (GvisorPlatform()) { case Platform::kKVM: - // TODO: See above. + // TODO(b/111805002): See above. return; case Platform::kPtrace: - // TODO: See above. + // TODO(gvisor.dev/issue/167): See above. return; case Platform::kNative: break; @@ -123,10 +123,10 @@ TEST(Syscall32Bit, Sysenter) { TEST(Syscall32Bit, Syscall) { switch (GvisorPlatform()) { case Platform::kKVM: - // TODO: See above. + // TODO(b/111805002): See above. return; case Platform::kPtrace: - // TODO: See above. + // TODO(gvisor.dev/issue/167): See above. return; case Platform::kNative: break; @@ -207,7 +207,7 @@ void FarCall32() { TEST(Call32Bit, Disallowed) { switch (GvisorPlatform()) { case Platform::kKVM: - // TODO: See above. + // TODO(b/111805002): See above. return; case Platform::kPtrace: // The ptrace platform cannot prevent switching to compatibility mode. diff --git a/test/syscalls/linux/aio.cc b/test/syscalls/linux/aio.cc index 06643ccb8..b96aab9b9 100644 --- a/test/syscalls/linux/aio.cc +++ b/test/syscalls/linux/aio.cc @@ -103,7 +103,7 @@ TEST_F(AIOTest, BasicWrite) { // aio implementation uses aio_ring. gVisor doesn't and returns all zeroes. // Linux implements aio_ring, so skip the zeroes check. // - // TODO: Remove when gVisor implements aio_ring. + // TODO(b/65486370): Remove when gVisor implements aio_ring. auto ring = reinterpret_cast(ctx_); auto magic = IsRunningOnGvisor() ? 0 : AIO_RING_MAGIC; EXPECT_EQ(ring->magic, magic); diff --git a/test/syscalls/linux/chmod.cc b/test/syscalls/linux/chmod.cc index 2f2ff3b7d..2f42fe326 100644 --- a/test/syscalls/linux/chmod.cc +++ b/test/syscalls/linux/chmod.cc @@ -235,7 +235,7 @@ TEST(ChmodTest, FchmodFileToNoPermissionsSucceeds_NoRandomSave) { // Verify that we can get a RW FD after chmod, even if a RO fd is left open. TEST(ChmodTest, ChmodWritableWithOpenFD) { - // FIXME: broken on hostfs. + // FIXME(b/72455313): broken on hostfs. if (IsRunningOnGvisor()) { return; } diff --git a/test/syscalls/linux/epoll.cc b/test/syscalls/linux/epoll.cc index 7b1d83ad8..b4a3bfcba 100644 --- a/test/syscalls/linux/epoll.cc +++ b/test/syscalls/linux/epoll.cc @@ -56,7 +56,7 @@ TEST(EpollTest, AllWritable) { struct epoll_event result[kFDsPerEpoll]; ASSERT_THAT(RetryEINTR(epoll_wait)(epollfd.get(), result, kFDsPerEpoll, -1), SyscallSucceedsWithValue(kFDsPerEpoll)); - // TODO: Why do some tests check epoll_event::data, and others + // TODO(edahlgren): Why do some tests check epoll_event::data, and others // don't? Does Linux actually guarantee that, in any of these test cases, // epoll_wait will necessarily write out the epoll_events in the order that // they were registered? diff --git a/test/syscalls/linux/exec_binary.cc b/test/syscalls/linux/exec_binary.cc index 187696ed9..c10d85398 100644 --- a/test/syscalls/linux/exec_binary.cc +++ b/test/syscalls/linux/exec_binary.cc @@ -285,7 +285,7 @@ ElfBinary<64> StandardElf() { elf.header.e_phoff = sizeof(elf.header); elf.header.e_phentsize = sizeof(decltype(elf)::ElfPhdr); - // TODO: Always include a PT_GNU_STACK segment to + // TODO(gvisor.dev/issue/153): Always include a PT_GNU_STACK segment to // disable executable stacks. With this omitted the stack (and all PROT_READ) // mappings should be executable, but gVisor doesn't support that. decltype(elf)::ElfPhdr phdr = {}; @@ -403,7 +403,7 @@ TEST(ElfTest, DataSegment) { // Linux will allow PT_LOAD segments to overlap. TEST(ElfTest, DirectlyOverlappingSegments) { - // NOTE: see PIEOutOfOrderSegments. + // NOTE(b/37289926): see PIEOutOfOrderSegments. SKIP_IF(IsRunningOnGvisor()); ElfBinary<64> elf = StandardElf(); @@ -439,7 +439,7 @@ TEST(ElfTest, DirectlyOverlappingSegments) { // Linux allows out-of-order PT_LOAD segments. TEST(ElfTest, OutOfOrderSegments) { - // NOTE: see PIEOutOfOrderSegments. + // NOTE(b/37289926): see PIEOutOfOrderSegments. SKIP_IF(IsRunningOnGvisor()); ElfBinary<64> elf = StandardElf(); @@ -670,7 +670,7 @@ TEST(ElfTest, PIENonZeroStart) { } TEST(ElfTest, PIEOutOfOrderSegments) { - // TODO: This triggers a bug in Linux where it computes the size + // TODO(b/37289926): This triggers a bug in Linux where it computes the size // of the binary as 0x20000 - 0x40000 = 0xfffffffffffe0000, which obviously // fails to map. // @@ -1005,7 +1005,7 @@ TEST(ElfTest, NoExecute) { // Execute, but no read permissions on the binary works just fine. TEST(ElfTest, NoRead) { - // TODO: gVisor's backing filesystem may prevent the + // TODO(gvisor.dev/issue/160): gVisor's backing filesystem may prevent the // sentry from reading the executable. SKIP_IF(IsRunningOnGvisor()); @@ -1024,7 +1024,7 @@ TEST(ElfTest, NoRead) { ASSERT_NO_ERRNO(WaitStopped(child)); - // TODO: A task with a non-readable executable is marked + // TODO(gvisor.dev/issue/160): A task with a non-readable executable is marked // non-dumpable, preventing access to proc files. gVisor does not implement // this behavior. } diff --git a/test/syscalls/linux/file_base.h b/test/syscalls/linux/file_base.h index 19c9a5053..43f568111 100644 --- a/test/syscalls/linux/file_base.h +++ b/test/syscalls/linux/file_base.h @@ -52,7 +52,7 @@ class FileTest : public ::testing::Test { test_file_fd_ = ASSERT_NO_ERRNO_AND_VALUE( Open(test_file_name_, O_CREAT | O_RDWR, S_IRUSR | S_IWUSR)); - // FIXME: enable when mknod syscall is supported. + // FIXME(edahlgren): enable when mknod syscall is supported. // test_fifo_name_ = NewTempAbsPath(); // ASSERT_THAT(mknod(test_fifo_name_.c_str()), S_IFIFO|0644, 0, // SyscallSucceeds()); @@ -97,7 +97,7 @@ class FileTest : public ::testing::Test { UnlinkFile(); ClosePipes(); - // FIXME: enable when mknod syscall is supported. + // FIXME(edahlgren): enable when mknod syscall is supported. // close(test_fifo_[0]); // close(test_fifo_[1]); // unlink(test_fifo_name_.c_str()); diff --git a/test/syscalls/linux/ioctl.cc b/test/syscalls/linux/ioctl.cc index de29047e0..c7741a177 100644 --- a/test/syscalls/linux/ioctl.cc +++ b/test/syscalls/linux/ioctl.cc @@ -158,7 +158,7 @@ TEST_F(IoctlTest, FIOASYNCNoTarget) { } TEST_F(IoctlTest, FIOASYNCSelfTarget) { - // FIXME: gVisor erroneously sends SIGIO on close(2), which would + // FIXME(b/120624367): gVisor erroneously sends SIGIO on close(2), which would // kill the test when pair goes out of scope. Temporarily ignore SIGIO so that // that the close signal is ignored. struct sigaction sa; @@ -195,7 +195,7 @@ TEST_F(IoctlTest, FIOASYNCSelfTarget) { // Equivalent to FIOASYNCSelfTarget except that FIOSETOWN is called before // FIOASYNC. TEST_F(IoctlTest, FIOASYNCSelfTarget2) { - // FIXME: gVisor erroneously sends SIGIO on close(2), which would + // FIXME(b/120624367): gVisor erroneously sends SIGIO on close(2), which would // kill the test when pair goes out of scope. Temporarily ignore SIGIO so that // that the close signal is ignored. struct sigaction sa; diff --git a/test/syscalls/linux/ip_socket_test_util.cc b/test/syscalls/linux/ip_socket_test_util.cc index 4ad787cc0..0a149c2e5 100644 --- a/test/syscalls/linux/ip_socket_test_util.cc +++ b/test/syscalls/linux/ip_socket_test_util.cc @@ -24,7 +24,7 @@ namespace gvisor { namespace testing { PosixErrorOr InterfaceIndex(std::string name) { - // TODO: Consider using netlink. + // TODO(igudger): Consider using netlink. ifreq req = {}; memcpy(req.ifr_name, name.c_str(), name.size()); ASSIGN_OR_RETURN_ERRNO(auto sock, Socket(AF_INET, SOCK_DGRAM, 0)); diff --git a/test/syscalls/linux/lseek.cc b/test/syscalls/linux/lseek.cc index fb6a1546e..6a4f1423c 100644 --- a/test/syscalls/linux/lseek.cc +++ b/test/syscalls/linux/lseek.cc @@ -194,7 +194,7 @@ TEST(LseekTest, EtcPasswdDup) { ASSERT_THAT(lseek(fd3.get(), 0, SEEK_CUR), SyscallSucceedsWithValue(1000)); } -// TODO: Add tests where we have donated in sockets. +// TODO(magi): Add tests where we have donated in sockets. } // namespace diff --git a/test/syscalls/linux/mkdir.cc b/test/syscalls/linux/mkdir.cc index 84db45eb3..50807b68f 100644 --- a/test/syscalls/linux/mkdir.cc +++ b/test/syscalls/linux/mkdir.cc @@ -36,7 +36,7 @@ class MkdirTest : public ::testing::Test { // TearDown unlinks created files. void TearDown() override { - // FIXME: We don't currently implement rmdir. + // FIXME(edahlgren): We don't currently implement rmdir. // We do this unconditionally because there's no harm in trying. rmdir(dirname_.c_str()); } diff --git a/test/syscalls/linux/mmap.cc b/test/syscalls/linux/mmap.cc index b500e79a4..a4fb9d1e0 100644 --- a/test/syscalls/linux/mmap.cc +++ b/test/syscalls/linux/mmap.cc @@ -816,7 +816,7 @@ class MMapFileTest : public MMapTest { // MAP_POPULATE allowed. // There isn't a good way to verify it actually did anything. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, MapPopulate) { ASSERT_THAT( Map(0, kPageSize, PROT_READ, MAP_PRIVATE | MAP_POPULATE, fd_.get(), 0), @@ -825,7 +825,7 @@ TEST_F(MMapFileTest, MapPopulate) { // MAP_POPULATE on a short file. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, MapPopulateShort) { ASSERT_THAT(Map(0, 2 * kPageSize, PROT_READ, MAP_PRIVATE | MAP_POPULATE, fd_.get(), 0), @@ -923,7 +923,7 @@ TEST_F(MMapFileTest, WriteSharedOnReadOnlyFd) { // MAP_SHARED PROT_READ not allowed on write-only FDs. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, ReadSharedOnWriteOnlyFd) { const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(filename_, O_WRONLY)); @@ -936,7 +936,7 @@ TEST_F(MMapFileTest, ReadSharedOnWriteOnlyFd) { // MAP_SHARED PROT_WRITE not allowed on write-only FDs. // The FD must always be readable. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, WriteSharedOnWriteOnlyFd) { const FileDescriptor fd = ASSERT_NO_ERRNO_AND_VALUE(Open(filename_, O_WRONLY)); @@ -1371,7 +1371,7 @@ TEST_F(MMapFileTest, WritePrivate) { // SIGBUS raised when writing past end of file to a private mapping. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, SigBusDeathWritePrivate) { SetupGvisorDeathTest(); @@ -1390,7 +1390,7 @@ TEST_F(MMapFileTest, SigBusDeathWritePrivate) { // SIGBUS raised when reading past end of file on a shared mapping. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, SigBusDeathReadShared) { SetupGvisorDeathTest(); @@ -1410,7 +1410,7 @@ TEST_F(MMapFileTest, SigBusDeathReadShared) { // SIGBUS raised when reading past end of file on a shared mapping. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, SigBusDeathWriteShared) { SetupGvisorDeathTest(); @@ -1459,7 +1459,7 @@ TEST_F(MMapFileTest, NoSigBusOnPageContainingEOFWritePrivate) { // Tests that SIGBUS is not raised when reading from a file-mapped page // containing EOF, *after* the EOF for a shared mapping. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, NoSigBusOnPageContainingEOFReadShared) { uintptr_t addr; ASSERT_THAT(addr = Map(0, 2 * kPageSize, PROT_READ, MAP_SHARED, fd_.get(), 0), @@ -1476,7 +1476,7 @@ TEST_F(MMapFileTest, NoSigBusOnPageContainingEOFReadShared) { // Tests that SIGBUS is not raised when writing to a file-mapped page containing // EOF, *after* the EOF for a shared mapping. // -// FIXME: Parameterize. +// FIXME(b/37222275): Parameterize. TEST_F(MMapFileTest, NoSigBusOnPageContainingEOFWriteShared) { uintptr_t addr; ASSERT_THAT(addr = Map(0, 2 * kPageSize, PROT_READ | PROT_WRITE, MAP_SHARED, diff --git a/test/syscalls/linux/open.cc b/test/syscalls/linux/open.cc index cdc226300..22e4666c2 100644 --- a/test/syscalls/linux/open.cc +++ b/test/syscalls/linux/open.cc @@ -279,7 +279,7 @@ TEST_F(OpenTest, Null) { ASSERT_THAT(open(&c, O_RDONLY), SyscallFailsWithErrno(ENOENT)); } -// NOTE: While the man pages specify that this behavior should be +// NOTE(b/119785738): While the man pages specify that this behavior should be // undefined, Linux truncates the file on opening read only if we have write // permission, so we will too. TEST_F(OpenTest, CanTruncateReadOnly) { diff --git a/test/syscalls/linux/partial_bad_buffer.cc b/test/syscalls/linux/partial_bad_buffer.cc index 073a6b8c1..71288ebc4 100644 --- a/test/syscalls/linux/partial_bad_buffer.cc +++ b/test/syscalls/linux/partial_bad_buffer.cc @@ -158,7 +158,7 @@ TEST_F(PartialBadBufferTest, PreadvSmall) { } TEST_F(PartialBadBufferTest, WriteBig) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -168,7 +168,7 @@ TEST_F(PartialBadBufferTest, WriteBig) { } TEST_F(PartialBadBufferTest, WriteSmall) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -178,7 +178,7 @@ TEST_F(PartialBadBufferTest, WriteSmall) { } TEST_F(PartialBadBufferTest, PwriteBig) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -188,7 +188,7 @@ TEST_F(PartialBadBufferTest, PwriteBig) { } TEST_F(PartialBadBufferTest, PwriteSmall) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -198,7 +198,7 @@ TEST_F(PartialBadBufferTest, PwriteSmall) { } TEST_F(PartialBadBufferTest, WritevBig) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -211,7 +211,7 @@ TEST_F(PartialBadBufferTest, WritevBig) { } TEST_F(PartialBadBufferTest, WritevSmall) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -224,7 +224,7 @@ TEST_F(PartialBadBufferTest, WritevSmall) { } TEST_F(PartialBadBufferTest, PwritevBig) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -238,7 +238,7 @@ TEST_F(PartialBadBufferTest, PwritevBig) { } TEST_F(PartialBadBufferTest, PwritevSmall) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); @@ -279,7 +279,7 @@ TEST_F(PartialBadBufferTest, GetdentsOneEntry) { // Verify that when write returns EFAULT the kernel hasn't silently written // the initial valid bytes. TEST_F(PartialBadBufferTest, WriteEfaultIsntPartial) { - // FIXME: The sentry write syscalls will return immediately + // FIXME(b/24788078): The sentry write syscalls will return immediately // if Access returns an error, but Access may not return an error // and the sentry will instead perform a partial write. SKIP_IF(IsRunningOnGvisor()); diff --git a/test/syscalls/linux/pipe.cc b/test/syscalls/linux/pipe.cc index c49ec9f09..abd10b11b 100644 --- a/test/syscalls/linux/pipe.cc +++ b/test/syscalls/linux/pipe.cc @@ -36,7 +36,7 @@ namespace { // Buffer size of a pipe. // -// TODO: Get this from F_GETPIPE_SZ. +// TODO(b/35762278): Get this from F_GETPIPE_SZ. constexpr int kPipeSize = 65536; class PipeTest : public ::testing::Test { @@ -316,7 +316,7 @@ TEST_F(PipeTest, BlockWriteClosed) { // Blocking write returns EPIPE when read end is closed even if something has // been written. // -// FIXME: Pipe writes blocking early allows S/R to interrupt the +// FIXME(b/35924046): Pipe writes blocking early allows S/R to interrupt the // write(2) call before the buffer is full. Then the next call will will return // non-zero instead of EPIPE. TEST_F(PipeTest, BlockPartialWriteClosed_NoRandomSave) { @@ -329,7 +329,7 @@ TEST_F(PipeTest, BlockPartialWriteClosed_NoRandomSave) { // Write more than fits in the buffer. Blocks then returns partial write // when the other end is closed. The next call returns EPIPE. if (IsRunningOnGvisor()) { - // FIXME: Pipe writes block early on gVisor, resulting in a + // FIXME(b/35924046): Pipe writes block early on gVisor, resulting in a // shorter than expected partial write. ASSERT_THAT(write(wfd, buf.data(), buf.size()), SyscallSucceedsWithValue(::testing::Gt(0))); diff --git a/test/syscalls/linux/proc.cc b/test/syscalls/linux/proc.cc index 3ec31ae8b..7ba274226 100644 --- a/test/syscalls/linux/proc.cc +++ b/test/syscalls/linux/proc.cc @@ -61,7 +61,7 @@ #include "test/util/thread_util.h" #include "test/util/timer_util.h" -// NOTE: No, this isn't really a syscall but this is a really simple +// NOTE(magi): No, this isn't really a syscall but this is a really simple // way to get it tested on both gVisor, PTrace and Linux. using ::testing::AllOf; @@ -489,7 +489,7 @@ TEST(ProcSelfMaps, Map1) { } TEST(ProcSelfMaps, Map2) { - // NOTE: The permissions must be different or the pages will get merged. + // NOTE(magi): The permissions must be different or the pages will get merged. Mapping map1 = ASSERT_NO_ERRNO_AND_VALUE( MmapAnon(kPageSize, PROT_READ | PROT_EXEC, MAP_PRIVATE)); Mapping map2 = @@ -564,7 +564,7 @@ TEST(ProcSelfMaps, MapUnmap) { } TEST(ProcSelfMaps, Mprotect) { - // FIXME: Linux's mprotect() sometimes fails to merge VMAs in this + // FIXME(jamieliu): Linux's mprotect() sometimes fails to merge VMAs in this // case. SKIP_IF(!IsRunningOnGvisor()); @@ -977,7 +977,7 @@ void MapPopulateRSS(int prot, uint64_t* before, uint64_t* after) { *after = ASSERT_NO_ERRNO_AND_VALUE(CurrentRSS()); } -// TODO: Test for PROT_READ + MAP_POPULATE anonymous mappings. Their +// TODO(b/73896574): Test for PROT_READ + MAP_POPULATE anonymous mappings. Their // semantics are more subtle: // // Small pages -> Zero page mapped, not counted in RSS @@ -1140,7 +1140,7 @@ TEST(ProcPidStatusTest, ValuesAreTabDelimited) { // Threads properly counts running threads. // -// TODO: Test zombied threads while the thread group leader is still +// TODO(mpratt): Test zombied threads while the thread group leader is still // running with generalized fork and clone children from the wait test. TEST(ProcPidStatusTest, Threads) { char buf[4096] = {}; @@ -1274,7 +1274,7 @@ TEST(ProcPidSymlink, SubprocessRunning) { SyscallSucceedsWithValue(sizeof(buf))); } -// FIXME: Inconsistent behavior between gVisor and linux +// FIXME(gvisor.dev/issue/164): Inconsistent behavior between gVisor and linux // on proc files. TEST(ProcPidSymlink, SubprocessZombied) { ASSERT_NO_ERRNO(SetCapability(CAP_DAC_OVERRIDE, false)); @@ -1298,13 +1298,13 @@ TEST(ProcPidSymlink, SubprocessZombied) { SyscallFailsWithErrno(want)); } - // FIXME: Inconsistent behavior between gVisor and linux + // FIXME(gvisor.dev/issue/164): Inconsistent behavior between gVisor and linux // on proc files. // 4.17 & gVisor: Syscall succeeds and returns 1 // EXPECT_THAT(ReadlinkWhileZombied("ns/pid", buf, sizeof(buf)), // SyscallFailsWithErrno(EACCES)); - // FIXME: Inconsistent behavior between gVisor and linux + // FIXME(gvisor.dev/issue/164): Inconsistent behavior between gVisor and linux // on proc files. // 4.17 & gVisor: Syscall succeeds and returns 1. // EXPECT_THAT(ReadlinkWhileZombied("ns/user", buf, sizeof(buf)), @@ -1313,7 +1313,7 @@ TEST(ProcPidSymlink, SubprocessZombied) { // Test whether /proc/PID/ symlinks can be read for an exited process. TEST(ProcPidSymlink, SubprocessExited) { - // FIXME: These all succeed on gVisor. + // FIXME(gvisor.dev/issue/164): These all succeed on gVisor. SKIP_IF(IsRunningOnGvisor()); char buf[1]; @@ -1404,7 +1404,7 @@ TEST(ProcPidFile, SubprocessZombie) { EXPECT_THAT(ReadWhileZombied("uid_map", buf, sizeof(buf)), SyscallSucceedsWithValue(sizeof(buf))); - // FIXME: Inconsistent behavior between gVisor and linux + // FIXME(gvisor.dev/issue/164): Inconsistent behavior between gVisor and linux // on proc files. // gVisor & 4.17: Succeeds and returns 1. // EXPECT_THAT(ReadWhileZombied("io", buf, sizeof(buf)), @@ -1415,7 +1415,7 @@ TEST(ProcPidFile, SubprocessZombie) { TEST(ProcPidFile, SubprocessExited) { char buf[1]; - // FIXME: Inconsistent behavior between kernels + // FIXME(gvisor.dev/issue/164): Inconsistent behavior between kernels // gVisor: Fails with ESRCH. // 4.17: Succeeds and returns 1. // EXPECT_THAT(ReadWhileExited("auxv", buf, sizeof(buf)), @@ -1425,7 +1425,7 @@ TEST(ProcPidFile, SubprocessExited) { SyscallFailsWithErrno(ESRCH)); if (!IsRunningOnGvisor()) { - // FIXME: Succeeds on gVisor. + // FIXME(gvisor.dev/issue/164): Succeeds on gVisor. EXPECT_THAT(ReadWhileExited("comm", buf, sizeof(buf)), SyscallFailsWithErrno(ESRCH)); } @@ -1434,25 +1434,25 @@ TEST(ProcPidFile, SubprocessExited) { SyscallSucceedsWithValue(sizeof(buf))); if (!IsRunningOnGvisor()) { - // FIXME: Succeeds on gVisor. + // FIXME(gvisor.dev/issue/164): Succeeds on gVisor. EXPECT_THAT(ReadWhileExited("io", buf, sizeof(buf)), SyscallFailsWithErrno(ESRCH)); } if (!IsRunningOnGvisor()) { - // FIXME: Returns EOF on gVisor. + // FIXME(gvisor.dev/issue/164): Returns EOF on gVisor. EXPECT_THAT(ReadWhileExited("maps", buf, sizeof(buf)), SyscallFailsWithErrno(ESRCH)); } if (!IsRunningOnGvisor()) { - // FIXME: Succeeds on gVisor. + // FIXME(gvisor.dev/issue/164): Succeeds on gVisor. EXPECT_THAT(ReadWhileExited("stat", buf, sizeof(buf)), SyscallFailsWithErrno(ESRCH)); } if (!IsRunningOnGvisor()) { - // FIXME: Succeeds on gVisor. + // FIXME(gvisor.dev/issue/164): Succeeds on gVisor. EXPECT_THAT(ReadWhileExited("status", buf, sizeof(buf)), SyscallFailsWithErrno(ESRCH)); } diff --git a/test/syscalls/linux/proc_pid_smaps.cc b/test/syscalls/linux/proc_pid_smaps.cc index 5f9c42ce5..cf5c462f3 100644 --- a/test/syscalls/linux/proc_pid_smaps.cc +++ b/test/syscalls/linux/proc_pid_smaps.cc @@ -82,7 +82,7 @@ struct ProcPidSmapsEntry { // Given the value part of a /proc/[pid]/smaps field containing a value in kB // (for example, " 4 kB", returns the value in kB (in this example, 4). PosixErrorOr SmapsValueKb(absl::string_view value) { - // TODO: let us use RE2 or + // TODO(jamieliu): let us use RE2 or std::pair parts = absl::StrSplit(value, ' ', absl::SkipEmpty()); if (parts.second != "kB") { diff --git a/test/syscalls/linux/ptrace.cc b/test/syscalls/linux/ptrace.cc index 1c9d7d4f4..e0c56f1fc 100644 --- a/test/syscalls/linux/ptrace.cc +++ b/test/syscalls/linux/ptrace.cc @@ -823,7 +823,7 @@ TEST(PtraceTest, TEST(PtraceTest, Int3) { switch (GvisorPlatform()) { case Platform::kKVM: - // TODO: int3 isn't handled properly. + // TODO(b/124248694): int3 isn't handled properly. return; default: break; diff --git a/test/syscalls/linux/pwrite64.cc b/test/syscalls/linux/pwrite64.cc index 60ae6de1f..485b1e48d 100644 --- a/test/syscalls/linux/pwrite64.cc +++ b/test/syscalls/linux/pwrite64.cc @@ -30,7 +30,7 @@ namespace { // This test is currently very rudimentary. // -// TODO: +// TODO(edahlgren): // * bad buffer states (EFAULT). // * bad fds (wrong permission, wrong type of file, EBADF). // * check offset is not incremented. diff --git a/test/syscalls/linux/readv_socket.cc b/test/syscalls/linux/readv_socket.cc index 2c129b7e8..cf22c395e 100644 --- a/test/syscalls/linux/readv_socket.cc +++ b/test/syscalls/linux/readv_socket.cc @@ -41,7 +41,7 @@ class ReadvSocketTest : public SocketTest { ASSERT_THAT(write(test_unix_seqpacket_socket_[1], kReadvTestData, kReadvTestDataSize), SyscallSucceedsWithValue(kReadvTestDataSize)); - // FIXME: Enable when possible. + // FIXME(b/69821513): Enable when possible. // ASSERT_THAT(write(test_tcp_socket_[1], kReadvTestData, // kReadvTestDataSize), // SyscallSucceedsWithValue(kReadvTestDataSize)); diff --git a/test/syscalls/linux/rtsignal.cc b/test/syscalls/linux/rtsignal.cc index 1f2fed7cc..ff948f9d5 100644 --- a/test/syscalls/linux/rtsignal.cc +++ b/test/syscalls/linux/rtsignal.cc @@ -75,7 +75,7 @@ class RtSignalTest : public ::testing::Test { static int rt_sigqueueinfo(pid_t tgid, int sig, siginfo_t* uinfo) { int ret; do { - // NOTE: rt_sigqueueinfo(2) could return EAGAIN for RT signals. + // NOTE(b/25434735): rt_sigqueueinfo(2) could return EAGAIN for RT signals. ret = syscall(SYS_rt_sigqueueinfo, tgid, sig, uinfo); } while (ret == -1 && errno == EAGAIN); return ret; diff --git a/test/syscalls/linux/socket_inet_loopback.cc b/test/syscalls/linux/socket_inet_loopback.cc index cdc5c0ce8..14d7827c2 100644 --- a/test/syscalls/linux/socket_inet_loopback.cc +++ b/test/syscalls/linux/socket_inet_loopback.cc @@ -221,7 +221,7 @@ TEST_P(SocketInetReusePortTest, TcpPortReuseMultiThread) { std::atomic connects_received = ATOMIC_VAR_INIT(0); std::unique_ptr listen_thread[kThreadCount]; int accept_counts[kThreadCount] = {}; - // TODO: figure how to not disable S/R for the whole test. + // TODO(avagin): figure how to not disable S/R for the whole test. // We need to take into account that this test executes a lot of system // calls from many threads. DisableSave ds; @@ -325,7 +325,7 @@ TEST_P(SocketInetReusePortTest, UdpPortReuseMultiThread) { std::atomic packets_received = ATOMIC_VAR_INIT(0); std::unique_ptr receiver_thread[kThreadCount]; int packets_per_socket[kThreadCount] = {}; - // TODO: figure how to not disable S/R for the whole test. + // TODO(avagin): figure how to not disable S/R for the whole test. DisableSave ds; // Too expensive. for (int i = 0; i < kThreadCount; i++) { @@ -642,7 +642,7 @@ TEST_P(SocketMultiProtocolInetLoopbackTest, V6OnlyV6AnyReservesV6) { TEST_P(SocketMultiProtocolInetLoopbackTest, V6EphemeralPortReserved) { auto const& param = GetParam(); - // FIXME + // FIXME(b/114268588) SKIP_IF(IsRunningOnGvisor() && param.type == SOCK_STREAM); for (int i = 0; true; i++) { @@ -743,7 +743,7 @@ TEST_P(SocketMultiProtocolInetLoopbackTest, V6EphemeralPortReserved) { TEST_P(SocketMultiProtocolInetLoopbackTest, V4MappedEphemeralPortReserved) { auto const& param = GetParam(); - // FIXME + // FIXME(b/114268588) SKIP_IF(IsRunningOnGvisor() && param.type == SOCK_STREAM); for (int i = 0; true; i++) { @@ -867,7 +867,7 @@ TEST_P(SocketMultiProtocolInetLoopbackTest, V4MappedEphemeralPortReserved) { TEST_P(SocketMultiProtocolInetLoopbackTest, V4EphemeralPortReserved) { auto const& param = GetParam(); - // FIXME + // FIXME(b/114268588) SKIP_IF(IsRunningOnGvisor() && param.type == SOCK_STREAM); for (int i = 0; true; i++) { diff --git a/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc b/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc index 8b4fc57b6..9dd9e1bd6 100644 --- a/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc +++ b/test/syscalls/linux/socket_ipv4_udp_unbound_external_networking.cc @@ -244,7 +244,7 @@ TestAddress V4Multicast() { // set interface or group membership. TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, TestSendMulticastSelfNoGroup) { - // FIXME: A group membership is not required for external + // FIXME(b/125485338): A group membership is not required for external // multicast on gVisor. SKIP_IF(IsRunningOnGvisor()); @@ -371,7 +371,7 @@ TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, // Check that multicast packets won't be delivered to another socket with no // set interface or group membership. TEST_P(IPv4UDPUnboundExternalNetworkingSocketTest, TestSendMulticastNoGroup) { - // FIXME: A group membership is not required for external + // FIXME(b/125485338): A group membership is not required for external // multicast on gVisor. SKIP_IF(IsRunningOnGvisor()); diff --git a/test/syscalls/linux/socket_netlink_route.cc b/test/syscalls/linux/socket_netlink_route.cc index 8d2e7d333..ed4ae1c71 100644 --- a/test/syscalls/linux/socket_netlink_route.cc +++ b/test/syscalls/linux/socket_netlink_route.cc @@ -180,7 +180,7 @@ void CheckGetLinkResponse(const struct nlmsghdr* hdr, int seq, int port) { // RTM_NEWLINK contains at least the header and ifinfomsg. EXPECT_GE(hdr->nlmsg_len, NLMSG_SPACE(sizeof(struct ifinfomsg))); - // TODO: Check ifinfomsg contents and following attrs. + // TODO(mpratt): Check ifinfomsg contents and following attrs. } TEST(NetlinkRouteTest, GetLinkDump) { @@ -370,7 +370,7 @@ TEST(NetlinkRouteTest, GetAddrDump) { // RTM_NEWADDR contains at least the header and ifaddrmsg. EXPECT_GE(hdr->nlmsg_len, sizeof(*hdr) + sizeof(struct ifaddrmsg)); - // TODO: Check ifaddrmsg contents and following attrs. + // TODO(mpratt): Check ifaddrmsg contents and following attrs. })); } diff --git a/test/syscalls/linux/socket_stream_blocking.cc b/test/syscalls/linux/socket_stream_blocking.cc index 8b3f6a647..f0f86c01c 100644 --- a/test/syscalls/linux/socket_stream_blocking.cc +++ b/test/syscalls/linux/socket_stream_blocking.cc @@ -33,7 +33,7 @@ namespace gvisor { namespace testing { TEST_P(BlockingStreamSocketPairTest, BlockPartialWriteClosed) { - // FIXME: gVisor doesn't support SO_SNDBUF on UDS, nor does it + // FIXME(b/35921550): gVisor doesn't support SO_SNDBUF on UDS, nor does it // enforce any limit; it will write arbitrary amounts of data without // blocking. SKIP_IF(IsRunningOnGvisor()); diff --git a/test/syscalls/linux/socket_test_util.cc b/test/syscalls/linux/socket_test_util.cc index 035087566..0be23e541 100644 --- a/test/syscalls/linux/socket_test_util.cc +++ b/test/syscalls/linux/socket_test_util.cc @@ -353,7 +353,7 @@ PosixErrorOr> CreateTCPAcceptBindSocketPair( } MaybeSave(); // Successful accept. - // FIXME + // FIXME(b/110484944) if (connect_result == -1) { absl::SleepFor(absl::Seconds(1)); } diff --git a/test/syscalls/linux/socket_unix.cc b/test/syscalls/linux/socket_unix.cc index 7332b768e..fafb23ad1 100644 --- a/test/syscalls/linux/socket_unix.cc +++ b/test/syscalls/linux/socket_unix.cc @@ -186,7 +186,7 @@ TEST_P(UnixSocketPairTest, BasicFDPassNoSpace) { // BasicFDPassNoSpaceMsgCtrunc sends an FD, but does not provide any space to // receive it. It then verifies that the MSG_CTRUNC flag is set in the msghdr. TEST_P(UnixSocketPairTest, BasicFDPassNoSpaceMsgCtrunc) { - // FIXME: Support MSG_CTRUNC. + // FIXME(gvisor.dev/issue/206): Support MSG_CTRUNC. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -224,7 +224,7 @@ TEST_P(UnixSocketPairTest, BasicFDPassNoSpaceMsgCtrunc) { // accomidate the FD, but msg_control is set to NULL. In this case, msg_control // should override msg_controllen. TEST_P(UnixSocketPairTest, BasicFDPassNullControlMsgCtrunc) { - // FIXME: Fix handling of NULL msg_control. + // FIXME(gvisor.dev/issue/207): Fix handling of NULL msg_control. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -259,7 +259,7 @@ TEST_P(UnixSocketPairTest, BasicFDPassNullControlMsgCtrunc) { // space to receive it. It then verifies that the MSG_CTRUNC flag is set in the // msghdr. TEST_P(UnixSocketPairTest, BasicFDPassNotEnoughSpaceMsgCtrunc) { - // FIXME: Support MSG_CTRUNC. + // FIXME(gvisor.dev/issue/206): Support MSG_CTRUNC. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -296,7 +296,7 @@ TEST_P(UnixSocketPairTest, BasicFDPassNotEnoughSpaceMsgCtrunc) { // space to receive two of them. It then verifies that the MSG_CTRUNC flag is // set in the msghdr. TEST_P(UnixSocketPairTest, BasicThreeFDPassTruncationMsgCtrunc) { - // FIXME: Support MSG_CTRUNC. + // FIXME(gvisor.dev/issue/206): Support MSG_CTRUNC. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -408,7 +408,7 @@ TEST_P(UnixSocketPairTest, BasicFDPassUnalignedRecvNoMsgTrunc) { // provides enough space to receive one of them. It then verifies that the // MSG_CTRUNC flag is set in the msghdr. TEST_P(UnixSocketPairTest, BasicTwoFDPassUnalignedRecvTruncationMsgTrunc) { - // FIXME: Support MSG_CTRUNC. + // FIXME(gvisor.dev/issue/206): Support MSG_CTRUNC. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -1010,7 +1010,7 @@ TEST_P(UnixSocketPairTest, CredPassNoMsgCtrunc) { // the data without providing space for any credentials and verifies that // MSG_CTRUNC is set in the msghdr. TEST_P(UnixSocketPairTest, CredPassNoSpaceMsgCtrunc) { - // FIXME: Support MSG_CTRUNC. + // FIXME(gvisor.dev/issue/206): Support MSG_CTRUNC. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -1061,7 +1061,7 @@ TEST_P(UnixSocketPairTest, CredPassNoSpaceMsgCtrunc) { // the data while providing enough space for only the first field of the // credentials and verifies that MSG_CTRUNC is set in the msghdr. TEST_P(UnixSocketPairTest, CredPassTruncatedMsgCtrunc) { - // FIXME: Support MSG_CTRUNC. + // FIXME(gvisor.dev/issue/206): Support MSG_CTRUNC. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); @@ -1615,7 +1615,7 @@ TEST_P(UnixSocketPairTest, SocketShutdown) { } TEST_P(UnixSocketPairTest, SocketReopenFromProcfs) { - // TODO: We should be returning ENXIO and NOT EIO. + // TODO(b/122310852): We should be returning ENXIO and NOT EIO. SKIP_IF(IsRunningOnGvisor()); auto sockets = ASSERT_NO_ERRNO_AND_VALUE(NewSocketPair()); diff --git a/test/syscalls/linux/socket_unix_dgram.cc b/test/syscalls/linux/socket_unix_dgram.cc index c17d3990f..5dd5e6d77 100644 --- a/test/syscalls/linux/socket_unix_dgram.cc +++ b/test/syscalls/linux/socket_unix_dgram.cc @@ -28,7 +28,7 @@ namespace testing { namespace { TEST_P(DgramUnixSocketPairTest, WriteOneSideClosed) { - // FIXME: gVisor datagram sockets return EPIPE instead of + // FIXME(b/35925052): gVisor datagram sockets return EPIPE instead of // ECONNREFUSED. SKIP_IF(IsRunningOnGvisor()); diff --git a/test/syscalls/linux/socket_unix_dgram_non_blocking.cc b/test/syscalls/linux/socket_unix_dgram_non_blocking.cc index 460eb8320..3becb513d 100644 --- a/test/syscalls/linux/socket_unix_dgram_non_blocking.cc +++ b/test/syscalls/linux/socket_unix_dgram_non_blocking.cc @@ -31,7 +31,7 @@ using NonBlockingDgramUnixSocketPairTest = SocketPairTest; TEST_P(NonBlockingDgramUnixSocketPairTest, ReadOneSideClosed) { if (IsRunningOnGvisor()) { - // FIXME: gVisor datagram sockets return 0 instead of + // FIXME(b/70803293): gVisor datagram sockets return 0 instead of // EAGAIN. return; } diff --git a/test/syscalls/linux/socket_unix_non_stream.cc b/test/syscalls/linux/socket_unix_non_stream.cc index 8e0cbee4c..a565978f9 100644 --- a/test/syscalls/linux/socket_unix_non_stream.cc +++ b/test/syscalls/linux/socket_unix_non_stream.cc @@ -47,7 +47,7 @@ TEST_P(UnixNonStreamSocketPairTest, RecvMsgTooLarge) { const int ret = RetryEINTR(write)(sockets->second_fd(), write_buf.data(), write_buf.size()); if (ret < 0 && errno == ENOBUFS) { - // NOTE: Linux may stall the write for a long time and + // NOTE(b/116636318): Linux may stall the write for a long time and // ultimately return ENOBUFS. Allow this error, since a retry will likely // result in the same error. return; @@ -136,7 +136,7 @@ TEST_P(UnixNonStreamSocketPairTest, FragmentedSendMsg) { // N.B. At minimum, the socketpair gofer should provide a socket that is // already the correct size. // - // TODO: When internal UDS support SO_SNDBUF, we can assert that + // TODO(b/35921550): When internal UDS support SO_SNDBUF, we can assert that // we always get the right SO_SNDBUF on gVisor. GTEST_SKIP() << "SO_SNDBUF = " << actual_sndbuf << ", want " << sndbuf; } @@ -156,7 +156,7 @@ TEST_P(UnixNonStreamSocketPairTest, FragmentedSendMsg) { msg.msg_iov = &iov; msg.msg_iovlen = 1; - // NOTE: Linux has poor behavior in the presence of + // NOTE(b/116636318,b/115833655): Linux has poor behavior in the presence of // physical memory fragmentation. As a result, this may stall for a long time // and ultimately return ENOBUFS. Allow this error, since it means that we // made it to the host kernel and started the sendmsg. @@ -192,7 +192,7 @@ TEST_P(UnixNonStreamSocketPairTest, FragmentedRecvMsg) { // N.B. At minimum, the socketpair gofer should provide a socket that is // already the correct size. // - // TODO: When internal UDS support SO_SNDBUF, we can assert that + // TODO(b/35921550): When internal UDS support SO_SNDBUF, we can assert that // we always get the right SO_SNDBUF on gVisor. GTEST_SKIP() << "SO_SNDBUF = " << actual_sndbuf << ", want " << sndbuf; } @@ -201,7 +201,7 @@ TEST_P(UnixNonStreamSocketPairTest, FragmentedRecvMsg) { const int ret = RetryEINTR(write)(sockets->first_fd(), write_buf.data(), write_buf.size()); if (ret < 0 && errno == ENOBUFS) { - // NOTE: Linux may stall the write for a long time and + // NOTE(b/116636318): Linux may stall the write for a long time and // ultimately return ENOBUFS. Allow this error, since a retry will likely // result in the same error. return; diff --git a/test/syscalls/linux/socket_unix_unbound_seqpacket.cc b/test/syscalls/linux/socket_unix_unbound_seqpacket.cc index 270d7203f..21209b244 100644 --- a/test/syscalls/linux/socket_unix_unbound_seqpacket.cc +++ b/test/syscalls/linux/socket_unix_unbound_seqpacket.cc @@ -42,7 +42,7 @@ TEST_P(UnboundUnixSeqpacketSocketPairTest, SendtoWithoutConnect) { } TEST_P(UnboundUnixSeqpacketSocketPairTest, SendtoWithoutConnectIgnoresAddr) { - // FIXME: gVisor tries to find /foo/bar and thus returns ENOENT. + // FIXME(b/68223466): gVisor tries to find /foo/bar and thus returns ENOENT. if (IsRunningOnGvisor()) { return; } diff --git a/test/syscalls/linux/socket_unix_unbound_stream.cc b/test/syscalls/linux/socket_unix_unbound_stream.cc index 4db5b4be1..b95f9569e 100644 --- a/test/syscalls/linux/socket_unix_unbound_stream.cc +++ b/test/syscalls/linux/socket_unix_unbound_stream.cc @@ -269,7 +269,7 @@ TEST_P(UnixStreamSocketPairTest, SinglePeek) { // 9f389e35674f5b086edd70ed524ca0f287259725 which changes this behavior. We // used to target 3.11 compatibility, so disable this test on newer kernels. // - // NOTE: Bring this up to Linux 4.4 compatibility. + // NOTE(b/118902768): Bring this up to Linux 4.4 compatibility. auto version = ASSERT_NO_ERRNO_AND_VALUE(GetKernelVersion()); SKIP_IF(version.major > 4 || (version.major == 4 && version.minor >= 3)); } @@ -686,7 +686,7 @@ TEST_P(UnboundUnixStreamSocketPairTest, SendtoWithoutConnect) { } TEST_P(UnboundUnixStreamSocketPairTest, SendtoWithoutConnectIgnoresAddr) { - // FIXME: gVisor tries to find /foo/bar and thus returns ENOENT. + // FIXME(b/68223466): gVisor tries to find /foo/bar and thus returns ENOENT. if (IsRunningOnGvisor()) { return; } diff --git a/test/syscalls/linux/stat.cc b/test/syscalls/linux/stat.cc index 48a2059de..746318d09 100644 --- a/test/syscalls/linux/stat.cc +++ b/test/syscalls/linux/stat.cc @@ -416,7 +416,7 @@ TEST_F(StatTest, ZeroLinksOpenFdRegularFileChild_NoRandomSave) { EXPECT_EQ(st_child_before.st_gid, st_child_fd.st_gid); EXPECT_EQ(st_child_before.st_size, st_child_fd.st_size); - // TODO: This isn't ideal but since fstatfs(2) will always return + // TODO(b/34861058): This isn't ideal but since fstatfs(2) will always return // OVERLAYFS_SUPER_MAGIC we have no way to know if this fs is backed by a // gofer which doesn't support links. EXPECT_TRUE(st_child_fd.st_nlink == 0 || st_child_fd.st_nlink == 1); diff --git a/test/syscalls/linux/stat_times.cc b/test/syscalls/linux/stat_times.cc index 442957c65..8346e9a8e 100644 --- a/test/syscalls/linux/stat_times.cc +++ b/test/syscalls/linux/stat_times.cc @@ -68,7 +68,7 @@ TEST_F(StatTimesTest, FileCreationTimes) { TEST_F(StatTimesTest, FileCtimeChanges) { auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile()); - MaybeSave(); // FIXME: ctime is inconsistent. + MaybeSave(); // FIXME(b/69865927): ctime is inconsistent. absl::Time atime, mtime, ctime; std::tie(atime, mtime, ctime) = GetTime(file); @@ -150,7 +150,7 @@ TEST_F(StatTimesTest, FileAtimeChanges) { const auto file = ASSERT_NO_ERRNO_AND_VALUE( TempPath::CreateFileWith(GetAbsoluteTestTmpdir(), contents, 0666)); - MaybeSave(); // FIXME: ctime is inconsistent. + MaybeSave(); // FIXME(b/69865927): ctime is inconsistent. absl::Time atime, mtime, ctime; std::tie(atime, mtime, ctime) = GetTime(file); @@ -184,7 +184,7 @@ TEST_F(StatTimesTest, DirAtimeChanges) { const auto file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path())); - MaybeSave(); // FIXME: ctime is inconsistent. + MaybeSave(); // FIXME(b/69865927): ctime is inconsistent. absl::Time atime, mtime, ctime; std::tie(atime, mtime, ctime) = GetTime(dir); @@ -193,7 +193,7 @@ TEST_F(StatTimesTest, DirAtimeChanges) { const absl::Time before = absl::Now() - absl::Seconds(1); - // NOTE: Keep an fd open. This ensures that the inode backing the + // NOTE(b/37756234): Keep an fd open. This ensures that the inode backing the // directory won't be destroyed before the final GetTime to avoid writing out // timestamps and causing side effects. const auto fd = ASSERT_NO_ERRNO_AND_VALUE(Open(dir.path(), O_RDONLY, 0)); diff --git a/test/syscalls/linux/tcp_socket.cc b/test/syscalls/linux/tcp_socket.cc index 1057f5892..33620a874 100644 --- a/test/syscalls/linux/tcp_socket.cc +++ b/test/syscalls/linux/tcp_socket.cc @@ -191,7 +191,7 @@ TEST_P(TcpSocketTest, SenderAddressIgnoredOnPeek) { TEST_P(TcpSocketTest, SendtoAddressIgnored) { struct sockaddr_storage addr; memset(&addr, 0, sizeof(addr)); - addr.ss_family = GetParam(); // FIXME + addr.ss_family = GetParam(); // FIXME(b/63803955) char data = '\0'; EXPECT_THAT( diff --git a/test/syscalls/linux/tkill.cc b/test/syscalls/linux/tkill.cc index 9842ccc9b..3e8ce5327 100644 --- a/test/syscalls/linux/tkill.cc +++ b/test/syscalls/linux/tkill.cc @@ -32,7 +32,7 @@ namespace { static int tkill(pid_t tid, int sig) { int ret; do { - // NOTE: tkill(2) could return EAGAIN for RT signals. + // NOTE(b/25434735): tkill(2) could return EAGAIN for RT signals. ret = syscall(SYS_tkill, tid, sig); } while (ret == -1 && errno == EAGAIN); return ret; diff --git a/test/syscalls/linux/udp_bind.cc b/test/syscalls/linux/udp_bind.cc index 902be47d3..547eb2a6c 100644 --- a/test/syscalls/linux/udp_bind.cc +++ b/test/syscalls/linux/udp_bind.cc @@ -286,7 +286,7 @@ INSTANTIATE_TEST_SUITE_P( []() { SendtoTestParam param = {}; param.description = "connected IPv6 sendto IPv4 mapped IPv6"; - // TODO: Determine if this inconsistent behavior is worth + // TODO(igudger): Determine if this inconsistent behavior is worth // implementing. param.skip_on_gvisor = true; param.send_domain = AF_INET6; @@ -299,7 +299,7 @@ INSTANTIATE_TEST_SUITE_P( []() { SendtoTestParam param = {}; param.description = "connected IPv6 sendto IPv4"; - // TODO: Determine if this inconsistent behavior is worth + // TODO(igudger): Determine if this inconsistent behavior is worth // implementing. param.skip_on_gvisor = true; param.send_domain = AF_INET6; diff --git a/test/syscalls/linux/uidgid.cc b/test/syscalls/linux/uidgid.cc index c0c1f2960..d78a09b1e 100644 --- a/test/syscalls/linux/uidgid.cc +++ b/test/syscalls/linux/uidgid.cc @@ -169,7 +169,7 @@ TEST(UidGidRootTest, SetgidNotFromThreadGroupLeader) { SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(IsRoot())); const gid_t gid = FLAGS_scratch_gid1; - // NOTE: Do setgid in a separate thread so that we can test if + // NOTE(b/64676707): Do setgid in a separate thread so that we can test if // info.si_pid is set correctly. ScopedThread([gid] { ASSERT_THAT(setgid(gid), SyscallSucceeds()); }); EXPECT_NO_ERRNO(CheckGIDs(gid, gid, gid)); diff --git a/test/syscalls/linux/utimes.cc b/test/syscalls/linux/utimes.cc index d95ee74ec..bf776cd93 100644 --- a/test/syscalls/linux/utimes.cc +++ b/test/syscalls/linux/utimes.cc @@ -33,7 +33,7 @@ namespace testing { namespace { -// TODO: utimes(nullptr) does not pick the "now" time in the +// TODO(b/36516566): utimes(nullptr) does not pick the "now" time in the // application's time domain, so when asserting that times are within a window, // we expand the window to allow for differences between the time domains. constexpr absl::Duration kClockSlack = absl::Milliseconds(100); @@ -235,7 +235,7 @@ void TestUtimensat(int dirFd, std::string const& path) { EXPECT_LE(mtime3, after); if (!IsRunningOnGvisor()) { - // FIXME: Gofers set atime and mtime to different "now" times. + // FIXME(b/36516566): Gofers set atime and mtime to different "now" times. EXPECT_EQ(atime3, mtime3); } } diff --git a/test/syscalls/linux/wait.cc b/test/syscalls/linux/wait.cc index cfab8a976..fcd606bec 100644 --- a/test/syscalls/linux/wait.cc +++ b/test/syscalls/linux/wait.cc @@ -40,7 +40,7 @@ using ::testing::UnorderedElementsAre; // These unit tests focus on the wait4(2) system call, but include a basic // checks for the i386 waitpid(2) syscall, which is a subset of wait4(2). // -// NOTE: Some functionality is not tested as +// NOTE(b/22640830,b/27680907,b/29049891): Some functionality is not tested as // it is not currently supported by gVisor: // * UID in waitid(2) siginfo. // * Process groups. diff --git a/test/syscalls/linux/write.cc b/test/syscalls/linux/write.cc index 432bd6066..7f80b2fa8 100644 --- a/test/syscalls/linux/write.cc +++ b/test/syscalls/linux/write.cc @@ -33,7 +33,7 @@ namespace testing { namespace { // This test is currently very rudimentary. // -// TODO: +// TODO(edahlgren): // * bad buffer states (EFAULT). // * bad fds (wrong permission, wrong type of file, EBADF). // * check offset is incremented. diff --git a/third_party/gvsync/downgradable_rwmutex_unsafe.go b/third_party/gvsync/downgradable_rwmutex_unsafe.go index a63a0d084..131f0a2ba 100644 --- a/third_party/gvsync/downgradable_rwmutex_unsafe.go +++ b/third_party/gvsync/downgradable_rwmutex_unsafe.go @@ -49,7 +49,7 @@ func (rw *DowngradableRWMutex) RLock() { // RUnlock undoes a single RLock call. func (rw *DowngradableRWMutex) RUnlock() { if RaceEnabled { - // TODO: Why does this need to be ReleaseMerge instead of + // TODO(jamieliu): Why does this need to be ReleaseMerge instead of // Release? IIUC this establishes Unlock happens-before RUnlock, which // seems unnecessary. RaceReleaseMerge(unsafe.Pointer(&rw.writerSem)) diff --git a/vdso/cycle_clock.h b/vdso/cycle_clock.h index 26d6690c0..309e07a3f 100644 --- a/vdso/cycle_clock.h +++ b/vdso/cycle_clock.h @@ -23,7 +23,7 @@ namespace vdso { #if __x86_64__ -// TODO: The appropriate barrier instruction to use with rdtsc on +// TODO(b/74613497): The appropriate barrier instruction to use with rdtsc on // x86_64 depends on the vendor. Intel processors can use lfence but AMD may // need mfence, depending on MSR_F10H_DECFG_LFENCE_SERIALIZE_BIT. diff --git a/vdso/vdso_amd64.lds b/vdso/vdso_amd64.lds index 166779931..e2615ae9e 100644 --- a/vdso/vdso_amd64.lds +++ b/vdso/vdso_amd64.lds @@ -56,7 +56,7 @@ SECTIONS { .altinstr_replacement : { *(.altinstr_replacement) } /* - * TODO: Remove this alignment? Then the VDSO would fit + * TODO(gvisor.dev/issue/157): Remove this alignment? Then the VDSO would fit * in a single page. */ . = ALIGN(0x1000); diff --git a/vdso/vdso_arm64.lds b/vdso/vdso_arm64.lds index 19f8efa01..469185468 100644 --- a/vdso/vdso_arm64.lds +++ b/vdso/vdso_arm64.lds @@ -59,7 +59,7 @@ SECTIONS { .altinstr_replacement : { *(.altinstr_replacement) } /* - * TODO: Remove this alignment? Then the VDSO would fit + * TODO(gvisor.dev/issue/157): Remove this alignment? Then the VDSO would fit * in a single page. */ . = ALIGN(0x1000);