diff options
| -rw-r--r-- | pkg/sentry/usermem/BUILD | 1 | ||||
| -rw-r--r-- | pkg/sentry/usermem/usermem.go | 14 | ||||
| -rw-r--r-- | pkg/sentry/usermem/usermem_unsafe.go | 27 |
3 files changed, 37 insertions, 5 deletions
diff --git a/pkg/sentry/usermem/BUILD b/pkg/sentry/usermem/BUILD index 1a560b6f3..e38b31b08 100644 --- a/pkg/sentry/usermem/BUILD +++ b/pkg/sentry/usermem/BUILD @@ -25,6 +25,7 @@ go_library( "bytes_io_unsafe.go", "usermem.go", "usermem_arm64.go", + "usermem_unsafe.go", "usermem_x86.go", ], importpath = "gvisor.googlesource.com/gvisor/pkg/sentry/usermem", diff --git a/pkg/sentry/usermem/usermem.go b/pkg/sentry/usermem/usermem.go index c3c9c153b..99766a803 100644 --- a/pkg/sentry/usermem/usermem.go +++ b/pkg/sentry/usermem/usermem.go @@ -37,6 +37,8 @@ type IO interface { // // Preconditions: The caller must not hold mm.MemoryManager.mappingMu or // any following locks in the lock order. + // + // Postconditions: CopyOut does not retain src. CopyOut(ctx context.Context, addr Addr, src []byte, opts IOOpts) (int, error) // CopyIn copies len(dst) bytes from the memory mapped at addr to dst. @@ -45,6 +47,8 @@ type IO interface { // // Preconditions: The caller must not hold mm.MemoryManager.mappingMu or // any following locks in the lock order. + // + // Postconditions: CopyIn does not retain dst. CopyIn(ctx context.Context, addr Addr, dst []byte, opts IOOpts) (int, error) // ZeroOut sets toZero bytes to 0, starting at addr. It returns the number @@ -237,7 +241,7 @@ func CopyStringIn(ctx context.Context, uio IO, addr Addr, maxlen int, opts IOOpt if !ok { // Last page of kernel memory. The application can't use this // anyway. - return string(buf[:done]), syserror.EFAULT + return stringFromImmutableBytes(buf[:done]), syserror.EFAULT } // Read up to copyStringIncrement bytes at a time. readlen := copyStringIncrement @@ -246,7 +250,7 @@ func CopyStringIn(ctx context.Context, uio IO, addr Addr, maxlen int, opts IOOpt } end, ok := start.AddLength(uint64(readlen)) if !ok { - return string(buf[:done]), syserror.EFAULT + return stringFromImmutableBytes(buf[:done]), syserror.EFAULT } // Shorten the read to avoid crossing page boundaries, since faulting // in a page unnecessarily is expensive. This also ensures that partial @@ -259,15 +263,15 @@ func CopyStringIn(ctx context.Context, uio IO, addr Addr, maxlen int, opts IOOpt // hitting err. for i, c := range buf[done : done+n] { if c == 0 { - return string(buf[:done+i]), nil + return stringFromImmutableBytes(buf[:done+i]), nil } } done += n if err != nil { - return string(buf[:done]), err + return stringFromImmutableBytes(buf[:done]), err } } - return string(buf), syserror.ENAMETOOLONG + return stringFromImmutableBytes(buf), syserror.ENAMETOOLONG } // CopyOutVec copies bytes from src to the memory mapped at ars in uio. The diff --git a/pkg/sentry/usermem/usermem_unsafe.go b/pkg/sentry/usermem/usermem_unsafe.go new file mode 100644 index 000000000..3895e7871 --- /dev/null +++ b/pkg/sentry/usermem/usermem_unsafe.go @@ -0,0 +1,27 @@ +// Copyright 2019 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package usermem + +import ( + "unsafe" +) + +// stringFromImmutableBytes is equivalent to string(bs), except that it never +// copies even if escape analysis can't prove that bs does not escape. This is +// only valid if bs is never mutated after stringFromImmutableBytes returns. +func stringFromImmutableBytes(bs []byte) string { + // Compare strings.Builder.String(). + return *(*string)(unsafe.Pointer(&bs)) +} |