Make {Un}Marshal{Bytes/Unsafe} return remaining buffer.

Change marshal.Marshallable method signatures to return the remaining buffer.
This makes it easier to implement these method manually. Without this, we would
have to manually do buffer shifting which is error prone.

tools/go_marshal/test:benchmark test does not show change in performance.
Additionally fixed some marshalling bugs in fsimpl/fuse.

Updated multiple callpoints to get rid of redundant slice indexing work and
simplified code using this new signature.

Updates #6450

PiperOrigin-RevId: 407857019

6 files changed, 24 insertions, 88 deletions

diff --git a/pkg/sentry/fsimpl/fuse/BUILD b/pkg/sentry/fsimpl/fuse/BUILD
index 05c4fbeb2..18497a880 100644
--- a/pkg/sentry/fsimpl/fuse/BUILD
+++ b/pkg/sentry/fsimpl/fuse/BUILD
@@ -77,8 +77,7 @@ go_test(
     deps = [
         "//pkg/abi/linux",
         "//pkg/errors/linuxerr",
-        "//pkg/hostarch",
-        "//pkg/marshal",
+        "//pkg/marshal/primitive",
         "//pkg/sentry/fsimpl/testutil",
         "//pkg/sentry/kernel",
         "//pkg/sentry/kernel/auth",
diff --git a/pkg/sentry/fsimpl/fuse/connection_test.go b/pkg/sentry/fsimpl/fuse/connection_test.go
index 1fddd858e..d98d2832b 100644
--- a/pkg/sentry/fsimpl/fuse/connection_test.go
+++ b/pkg/sentry/fsimpl/fuse/connection_test.go
@@ -20,6 +20,7 @@ import (
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
+	"gvisor.dev/gvisor/pkg/marshal/primitive"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 )
@@ -69,14 +70,10 @@ func TestConnectionAbort(t *testing.T) {
 		t.Fatalf("newTestConnection: %v", err)
 	}
 
-	testObj := &testPayload{
-		data: rand.Uint32(),
-	}
-
 	var futNormal []*futureResponse
-
+	testObj := primitive.Uint32(rand.Uint32())
 	for i := 0; i < int(numRequests); i++ {
-		req := conn.NewRequest(creds, uint32(i), uint64(i), 0, testObj)
+		req := conn.NewRequest(creds, uint32(i), uint64(i), 0, &testObj)
 		fut, err := conn.callFutureLocked(task, req)
 		if err != nil {
 			t.Fatalf("callFutureLocked failed: %v", err)
@@ -102,7 +99,7 @@ func TestConnectionAbort(t *testing.T) {
 	}
 
 	// After abort, Call() should return directly with ENOTCONN.
-	req := conn.NewRequest(creds, 0, 0, 0, testObj)
+	req := conn.NewRequest(creds, 0, 0, 0, &testObj)
 	_, err = conn.Call(task, req)
 	if !linuxerr.Equals(linuxerr.ENOTCONN, err) {
 		t.Fatalf("Incorrect error code received for Call() after connection aborted")
diff --git a/pkg/sentry/fsimpl/fuse/dev_test.go b/pkg/sentry/fsimpl/fuse/dev_test.go
index 8951b5ba8..13b32fc7c 100644
--- a/pkg/sentry/fsimpl/fuse/dev_test.go
+++ b/pkg/sentry/fsimpl/fuse/dev_test.go
@@ -21,6 +21,7 @@ import (
 
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
+	"gvisor.dev/gvisor/pkg/marshal/primitive"
 	"gvisor.dev/gvisor/pkg/sentry/fsimpl/testutil"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
@@ -215,11 +216,9 @@ func fuseClientRun(t *testing.T, s *testutil.System, k *kernel.Kernel, conn *con
 	if err != nil {
 		t.Fatal(err)
 	}
-	testObj := &testPayload{
-		data: rand.Uint32(),
-	}
 
-	req := conn.NewRequest(creds, pid, inode, echoTestOpcode, testObj)
+	testObj := primitive.Uint32(rand.Uint32())
+	req := conn.NewRequest(creds, pid, inode, echoTestOpcode, &testObj)
 
 	// Queue up a request.
 	// Analogous to Call except it doesn't block on the task.
@@ -232,7 +231,7 @@ func fuseClientRun(t *testing.T, s *testutil.System, k *kernel.Kernel, conn *con
 		t.Fatalf("Server responded with an error: %v", err)
 	}
 
-	var respTestPayload testPayload
+	var respTestPayload primitive.Uint32
 	if err := resp.UnmarshalPayload(&respTestPayload); err != nil {
 		t.Fatalf("Unmarshalling payload error: %v", err)
 	}
@@ -242,8 +241,8 @@ func fuseClientRun(t *testing.T, s *testutil.System, k *kernel.Kernel, conn *con
 			req.hdr.Unique, resp.hdr.Unique)
 	}
 
-	if respTestPayload.data != testObj.data {
-		t.Fatalf("read incorrect data. Data expected: %v, but got %v", testObj.data, respTestPayload.data)
+	if respTestPayload != testObj {
+		t.Fatalf("read incorrect data. Data expected: %d, but got %d", testObj, respTestPayload)
 	}
 
 }
@@ -256,8 +255,8 @@ func fuseServerRun(t *testing.T, s *testutil.System, k *kernel.Kernel, fd *vfs.F
 
 	// Create the tasks that the server will be using.
 	tc := k.NewThreadGroup(nil, k.RootPIDNamespace(), kernel.NewSignalHandlers(), linux.SIGCHLD, k.GlobalInit().Limits())
-	var readPayload testPayload
 
+	var readPayload primitive.Uint32
 	serverTask, err := testutil.CreateTask(s.Ctx, "fuse-server", tc, s.MntNs, s.Root, s.Root)
 	if err != nil {
 		t.Fatal(err)
@@ -291,8 +290,8 @@ func fuseServerRun(t *testing.T, s *testutil.System, k *kernel.Kernel, fd *vfs.F
 		}
 
 		var readFUSEHeaderIn linux.FUSEHeaderIn
-		readFUSEHeaderIn.UnmarshalUnsafe(inBuf[:inHdrLen])
-		readPayload.UnmarshalUnsafe(inBuf[inHdrLen : inHdrLen+payloadLen])
+		inBuf = readFUSEHeaderIn.UnmarshalUnsafe(inBuf)
+		readPayload.UnmarshalUnsafe(inBuf)
 
 		if readFUSEHeaderIn.Opcode != echoTestOpcode {
 			t.Fatalf("read incorrect data. Header: %v, Payload: %v", readFUSEHeaderIn, readPayload)
diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go
index af16098d2..00b520c31 100644
--- a/pkg/sentry/fsimpl/fuse/fusefs.go
+++ b/pkg/sentry/fsimpl/fuse/fusefs.go
@@ -489,7 +489,7 @@ func (i *inode) Open(ctx context.Context, rp *vfs.ResolvingPath, d *kernfs.Dentr
 
 // Lookup implements kernfs.Inode.Lookup.
 func (i *inode) Lookup(ctx context.Context, name string) (kernfs.Inode, error) {
-	in := linux.FUSELookupIn{Name: name}
+	in := linux.FUSELookupIn{Name: linux.CString(name)}
 	return i.newEntry(ctx, name, 0, linux.FUSE_LOOKUP, &in)
 }
 
@@ -520,7 +520,7 @@ func (i *inode) NewFile(ctx context.Context, name string, opts vfs.OpenOptions)
 			Mode:  uint32(opts.Mode) | linux.S_IFREG,
 			Umask: uint32(kernelTask.FSContext().Umask()),
 		},
-		Name: name,
+		Name: linux.CString(name),
 	}
 	return i.newEntry(ctx, name, linux.S_IFREG, linux.FUSE_CREATE, &in)
 }
@@ -533,7 +533,7 @@ func (i *inode) NewNode(ctx context.Context, name string, opts vfs.MknodOptions)
 			Rdev:  linux.MakeDeviceID(uint16(opts.DevMajor), opts.DevMinor),
 			Umask: uint32(kernel.TaskFromContext(ctx).FSContext().Umask()),
 		},
-		Name: name,
+		Name: linux.CString(name),
 	}
 	return i.newEntry(ctx, name, opts.Mode.FileType(), linux.FUSE_MKNOD, &in)
 }
@@ -541,8 +541,8 @@ func (i *inode) NewNode(ctx context.Context, name string, opts vfs.MknodOptions)
 // NewSymlink implements kernfs.Inode.NewSymlink.
 func (i *inode) NewSymlink(ctx context.Context, name, target string) (kernfs.Inode, error) {
 	in := linux.FUSESymLinkIn{
-		Name:   name,
-		Target: target,
+		Name:   linux.CString(name),
+		Target: linux.CString(target),
 	}
 	return i.newEntry(ctx, name, linux.S_IFLNK, linux.FUSE_SYMLINK, &in)
 }
@@ -554,7 +554,7 @@ func (i *inode) Unlink(ctx context.Context, name string, child kernfs.Inode) err
 		log.Warningf("fusefs.Inode.newEntry: couldn't get kernel task from context", i.nodeID)
 		return linuxerr.EINVAL
 	}
-	in := linux.FUSEUnlinkIn{Name: name}
+	in := linux.FUSEUnlinkIn{Name: linux.CString(name)}
 	req := i.fs.conn.NewRequest(auth.CredentialsFromContext(ctx), uint32(kernelTask.ThreadID()), i.nodeID, linux.FUSE_UNLINK, &in)
 	res, err := i.fs.conn.Call(kernelTask, req)
 	if err != nil {
@@ -571,7 +571,7 @@ func (i *inode) NewDir(ctx context.Context, name string, opts vfs.MkdirOptions)
 			Mode:  uint32(opts.Mode),
 			Umask: uint32(kernel.TaskFromContext(ctx).FSContext().Umask()),
 		},
-		Name: name,
+		Name: linux.CString(name),
 	}
 	return i.newEntry(ctx, name, linux.S_IFDIR, linux.FUSE_MKDIR, &in)
 }
@@ -581,7 +581,7 @@ func (i *inode) RmDir(ctx context.Context, name string, child kernfs.Inode) erro
 	fusefs := i.fs
 	task, creds := kernel.TaskFromContext(ctx), auth.CredentialsFromContext(ctx)
 
-	in := linux.FUSERmDirIn{Name: name}
+	in := linux.FUSERmDirIn{Name: linux.CString(name)}
 	req := fusefs.conn.NewRequest(creds, uint32(task.ThreadID()), i.nodeID, linux.FUSE_RMDIR, &in)
 	res, err := i.fs.conn.Call(task, req)
 	if err != nil {
diff --git a/pkg/sentry/fsimpl/fuse/request_response.go b/pkg/sentry/fsimpl/fuse/request_response.go
index 8a72489fa..ec76ec2a4 100644
--- a/pkg/sentry/fsimpl/fuse/request_response.go
+++ b/pkg/sentry/fsimpl/fuse/request_response.go
@@ -41,7 +41,7 @@ type fuseInitRes struct {
 }
 
 // UnmarshalBytes deserializes src to the initOut attribute in a fuseInitRes.
-func (r *fuseInitRes) UnmarshalBytes(src []byte) {
+func (r *fuseInitRes) UnmarshalBytes(src []byte) []byte {
 	out := &r.initOut
 
 	// Introduced before FUSE kernel version 7.13.
@@ -70,7 +70,7 @@ func (r *fuseInitRes) UnmarshalBytes(src []byte) {
 		out.MaxPages = uint16(hostarch.ByteOrder.Uint16(src[:2]))
 		src = src[2:]
 	}
-	_ = src // Remove unused warning.
+	return src
 }
 
 // SizeBytes is the size of the payload of the FUSE_INIT response.
diff --git a/pkg/sentry/fsimpl/fuse/utils_test.go b/pkg/sentry/fsimpl/fuse/utils_test.go
index b0bab0066..8d4a2fad3 100644
--- a/pkg/sentry/fsimpl/fuse/utils_test.go
+++ b/pkg/sentry/fsimpl/fuse/utils_test.go
@@ -15,17 +15,13 @@
 package fuse
 
 import (
-	"io"
 	"testing"
 
 	"gvisor.dev/gvisor/pkg/abi/linux"
-	"gvisor.dev/gvisor/pkg/marshal"
 	"gvisor.dev/gvisor/pkg/sentry/fsimpl/testutil"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
-
-	"gvisor.dev/gvisor/pkg/hostarch"
 )
 
 func setup(t *testing.T) *testutil.System {
@@ -70,58 +66,3 @@ func newTestConnection(system *testutil.System, k *kernel.Kernel, maxActiveReque
 	}
 	return fs.conn, &fuseDev.vfsfd, nil
 }
-
-type testPayload struct {
-	marshal.StubMarshallable
-	data uint32
-}
-
-// SizeBytes implements marshal.Marshallable.SizeBytes.
-func (t *testPayload) SizeBytes() int {
-	return 4
-}
-
-// MarshalBytes implements marshal.Marshallable.MarshalBytes.
-func (t *testPayload) MarshalBytes(dst []byte) {
-	hostarch.ByteOrder.PutUint32(dst[:4], t.data)
-}
-
-// UnmarshalBytes implements marshal.Marshallable.UnmarshalBytes.
-func (t *testPayload) UnmarshalBytes(src []byte) {
-	*t = testPayload{data: hostarch.ByteOrder.Uint32(src[:4])}
-}
-
-// Packed implements marshal.Marshallable.Packed.
-func (t *testPayload) Packed() bool {
-	return true
-}
-
-// MarshalUnsafe implements marshal.Marshallable.MarshalUnsafe.
-func (t *testPayload) MarshalUnsafe(dst []byte) {
-	t.MarshalBytes(dst)
-}
-
-// UnmarshalUnsafe implements marshal.Marshallable.UnmarshalUnsafe.
-func (t *testPayload) UnmarshalUnsafe(src []byte) {
-	t.UnmarshalBytes(src)
-}
-
-// CopyOutN implements marshal.Marshallable.CopyOutN.
-func (t *testPayload) CopyOutN(task marshal.CopyContext, addr hostarch.Addr, limit int) (int, error) {
-	panic("not implemented")
-}
-
-// CopyOut implements marshal.Marshallable.CopyOut.
-func (t *testPayload) CopyOut(task marshal.CopyContext, addr hostarch.Addr) (int, error) {
-	panic("not implemented")
-}
-
-// CopyIn implements marshal.Marshallable.CopyIn.
-func (t *testPayload) CopyIn(task marshal.CopyContext, addr hostarch.Addr) (int, error) {
-	panic("not implemented")
-}
-
-// WriteTo implements io.WriterTo.WriteTo.
-func (t *testPayload) WriteTo(w io.Writer) (int64, error) {
-	panic("not implemented")
-}