summaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorJamie Liu <jamieliu@google.com>2019-03-27 10:45:17 -0700
committerShentubot <shentubot@google.com>2019-03-27 10:46:28 -0700
commit26583e413e40666f70170025cd4c5224f45fdfa3 (patch)
treed77dc00d64a874fa6869b2ce58ba8673b1919ecc
parentbeb71ab681dadb2eed3407bc9188bfe85694eb22 (diff)
Convert []byte to string without copying in usermem.CopyStringIn.
This is the same technique used by Go's strings.Builder (https://golang.org/src/strings/builder.go#L45), and for the same reason. (We can't just use strings.Builder because there's no way to get the underlying []byte to pass to usermem.IO.CopyIn.) PiperOrigin-RevId: 240594892 Change-Id: Ic070e7e480aee53a71289c7c120850991358c52c
-rw-r--r--pkg/sentry/usermem/BUILD1
-rw-r--r--pkg/sentry/usermem/usermem.go14
-rw-r--r--pkg/sentry/usermem/usermem_unsafe.go27
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))
+}