diff --git a/pkg/sentry/fsimpl/proc/tasks_sys.go b/pkg/sentry/fsimpl/proc/tasks_sys.go index 8c31e0c070..3a2b64c7c1 100644 --- a/pkg/sentry/fsimpl/proc/tasks_sys.go +++ b/pkg/sentry/fsimpl/proc/tasks_sys.go @@ -227,22 +227,15 @@ func (d *tcpSackData) Write(ctx context.Context, _ *vfs.FileDescription, src use // No need to handle partial writes thus far. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } - - // Limit the amount of memory allocated. - src = src.TakeFirst(hostarch.PageSize - 1) - - var v int32 - n, err := usermem.CopyInt32StringInVec(ctx, src.IO, src.Addrs, &v, src.Opts) - if err != nil { + buf := make([]int32, 1) + n, err := ParseInt32Vec(ctx, src, buf) + if err != nil || n == 0 { return 0, err } if d.enabled == nil { d.enabled = new(bool) } - *d.enabled = v != 0 + *d.enabled = buf[0] != 0 return n, d.stack.SetTCPSACKEnabled(*d.enabled) } @@ -275,19 +268,12 @@ func (d *tcpRecoveryData) Write(ctx context.Context, _ *vfs.FileDescription, src // No need to handle partial writes thus far. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } - - // Limit the amount of memory allocated. - src = src.TakeFirst(hostarch.PageSize - 1) - - var v int32 - n, err := usermem.CopyInt32StringInVec(ctx, src.IO, src.Addrs, &v, src.Opts) - if err != nil { + buf := make([]int32, 1) + n, err := ParseInt32Vec(ctx, src, buf) + if err != nil || n == 0 { return 0, err } - if err := d.stack.SetTCPRecovery(inet.TCPLossRecovery(v)); err != nil { + if err := d.stack.SetTCPRecovery(inet.TCPLossRecovery(buf[0])); err != nil { return 0, err } return n, nil @@ -329,21 +315,15 @@ func (d *tcpMemData) Write(ctx context.Context, _ *vfs.FileDescription, src user // No need to handle partial writes thus far. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } d.mu.Lock() defer d.mu.Unlock() - - // Limit the amount of memory allocated. - src = src.TakeFirst(hostarch.PageSize - 1) size, err := d.readSizeLocked() if err != nil { return 0, err } buf := []int32{int32(size.Min), int32(size.Default), int32(size.Max)} - n, err := usermem.CopyInt32StringsInVec(ctx, src.IO, src.Addrs, buf, src.Opts) - if err != nil { + n, err := ParseInt32Vec(ctx, src, buf) + if err != nil || n == 0 { return 0, err } newSize := inet.TCPBufferSize{ @@ -414,19 +394,12 @@ func (ipf *ipForwarding) Write(ctx context.Context, _ *vfs.FileDescription, src // No need to handle partial writes thus far. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } - - // Limit input size so as not to impact performance if input size is large. - src = src.TakeFirst(hostarch.PageSize - 1) - - var v int32 - n, err := usermem.CopyInt32StringInVec(ctx, src.IO, src.Addrs, &v, src.Opts) - if err != nil { + buf := make([]int32, 1) + n, err := ParseInt32Vec(ctx, src, buf) + if err != nil || n == 0 { return 0, err } - ipf.enabled = v != 0 + ipf.enabled = buf[0] != 0 if err := ipf.stack.SetForwarding(ipv4.ProtocolNumber, ipf.enabled); err != nil { return 0, err } @@ -467,17 +440,10 @@ func (pr *portRange) Write(ctx context.Context, _ *vfs.FileDescription, src user // No need to handle partial writes thus far. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } - - // Limit input size so as not to impact performance if input size is - // large. - src = src.TakeFirst(hostarch.PageSize - 1) ports := make([]int32, 2) - n, err := usermem.CopyInt32StringsInVec(ctx, src.IO, src.Addrs, ports, src.Opts) - if err != nil { + n, err := ParseInt32Vec(ctx, src, ports) + if err != nil || n == 0 { return 0, err } @@ -523,24 +489,17 @@ func (f *atomicInt32File) Write(ctx context.Context, _ *vfs.FileDescription, src // Ignore partial writes. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } - - // Limit the amount of memory allocated. - src = src.TakeFirst(hostarch.PageSize - 1) - - var v int32 - n, err := usermem.CopyInt32StringInVec(ctx, src.IO, src.Addrs, &v, src.Opts) - if err != nil { + buf := make([]int32, 1) + n, err := ParseInt32Vec(ctx, src, buf) + if err != nil || n == 0 { return 0, err } - if v < f.min || v > f.max { + if buf[0] < f.min || buf[0] > f.max { return 0, linuxerr.EINVAL } - f.val.Store(v) + f.val.Store(buf[0]) return n, nil } @@ -555,3 +514,17 @@ func randUUID() string { uuid[6] = (uuid[6] & 0x0f) | 0x40 // Version 4 (random) return fmt.Sprintf("%x-%x-%x-%x-%x\n", uuid[:4], uuid[4:6], uuid[6:8], uuid[8:10], uuid[10:]) } + +// ParseInt32Vec interprets src as string encoding slice of int32, and +// returns the parsed value and the number of bytes read. +// +// The numbers of int32 will be populated even if an error is returned eventually. +func ParseInt32Vec(ctx context.Context, src usermem.IOSequence, buf []int32) (int64, error) { + if src.NumBytes() == 0 { + return 0, nil + } + // Limit input size so as not to impact performance if input size is large. + src = src.TakeFirst(hostarch.PageSize - 1) + + return usermem.CopyInt32StringsInVec(ctx, src.IO, src.Addrs, buf, src.Opts) +} diff --git a/pkg/sentry/fsimpl/proc/tasks_sys_test.go b/pkg/sentry/fsimpl/proc/tasks_sys_test.go index 96b3d8b7a9..06114e5e3a 100644 --- a/pkg/sentry/fsimpl/proc/tasks_sys_test.go +++ b/pkg/sentry/fsimpl/proc/tasks_sys_test.go @@ -17,6 +17,7 @@ package proc import ( "bytes" "reflect" + "slices" "testing" "gvisor.dev/gvisor/pkg/abi/linux" @@ -147,3 +148,59 @@ func TestConfigureIPForwarding(t *testing.T) { }) } } + +func TestParseInt32Vec(t *testing.T) { + ctx := context.Background() + tests := []struct { + name string + src []byte + buf []int32 + expectedBuf []int32 + expectedBytes int64 + }{ + { + name: "EmptyInput", + src: []byte(""), + buf: []int32{}, + expectedBuf: []int32{}, + expectedBytes: 0, + }, + { + name: "SingleNum", + src: []byte("123"), + buf: []int32{0}, + expectedBuf: []int32{123}, + expectedBytes: 3, + }, + { + name: "MultipleNum", + src: []byte("123 456"), + buf: []int32{0, 0}, + expectedBuf: []int32{123, 456}, + expectedBytes: 7, + }, + { + name: "FirstNNum", + src: []byte("123 456"), + buf: []int32{0, 0, 789}, + expectedBuf: []int32{123, 456, 789}, + expectedBytes: 7, + }, + { + name: "ParsePartially", + src: []byte("123 a"), + buf: []int32{0}, + expectedBuf: []int32{123}, + expectedBytes: 4, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + originalBuf := append([]int32(nil), test.buf...) + n, err := ParseInt32Vec(ctx, usermem.BytesIOSequence(test.src), test.buf) + if !slices.Equal(test.buf, test.expectedBuf) || n != test.expectedBytes { + t.Errorf("ParseInt32Vec(ctx, %v, %v) returns %v, %v, result buffer is %v; expected: %v, %v", test.src, originalBuf, n, err, test.buf, test.expectedBuf, test.expectedBytes) + } + }) + } +} diff --git a/pkg/sentry/fsimpl/proc/yama.go b/pkg/sentry/fsimpl/proc/yama.go index cc05061a1d..ed9e0b1547 100644 --- a/pkg/sentry/fsimpl/proc/yama.go +++ b/pkg/sentry/fsimpl/proc/yama.go @@ -22,7 +22,6 @@ import ( "gvisor.dev/gvisor/pkg/atomicbitops" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/errors/linuxerr" - "gvisor.dev/gvisor/pkg/hostarch" "gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -61,24 +60,17 @@ func (s *yamaPtraceScope) Write(ctx context.Context, _ *vfs.FileDescription, src // Ignore partial writes. return 0, linuxerr.EINVAL } - if src.NumBytes() == 0 { - return 0, nil - } - - // Limit the amount of memory allocated. - src = src.TakeFirst(hostarch.PageSize - 1) - - var v int32 - n, err := usermem.CopyInt32StringInVec(ctx, src.IO, src.Addrs, &v, src.Opts) - if err != nil { + buf := make([]int32, 1) + n, err := ParseInt32Vec(ctx, src, buf) + if err != nil || n == 0 { return 0, err } // We do not support YAMA levels > YAMA_SCOPE_RELATIONAL. - if v < linux.YAMA_SCOPE_DISABLED || v > linux.YAMA_SCOPE_RELATIONAL { + if buf[0] < linux.YAMA_SCOPE_DISABLED || buf[0] > linux.YAMA_SCOPE_RELATIONAL { return 0, linuxerr.EINVAL } - s.level.Store(v) + s.level.Store(buf[0]) return n, nil } diff --git a/pkg/sentry/vfs/file_description_impl_util.go b/pkg/sentry/vfs/file_description_impl_util.go index 1aedc9ca89..3c84551630 100644 --- a/pkg/sentry/vfs/file_description_impl_util.go +++ b/pkg/sentry/vfs/file_description_impl_util.go @@ -256,8 +256,6 @@ func (s *StaticData) Generate(ctx context.Context, buf *bytes.Buffer) error { // WritableDynamicBytesSource extends DynamicBytesSource to allow writes to the // underlying source. -// -// TODO(b/179825241): Make utility for integer-based writable files. type WritableDynamicBytesSource interface { DynamicBytesSource