Merge pull request google#4 from ridwanmsharif/ridwanmsharif/fix-read…

…dir-test

fuse: use safe go_marshal API for FUSE

pkg/abi/linux/fuse.go

@@ -359,7 +359,12 @@ type FUSELookupIn struct {
 
 // MarshalUnsafe serializes r.name to the dst buffer.
 func (r *FUSELookupIn) MarshalUnsafe(buf []byte) {
-	copy(buf, []byte(r.Name))
+	copy(buf, r.Name)
+}
+
+// MarshalBytes serializes r.name to the dst buffer.
+func (r *FUSELookupIn) MarshalBytes(buf []byte) {
+	copy(buf, r.Name)
 }
 
 // SizeBytes is the size of the memory representation of FUSELookupIn.
@@ -491,6 +496,12 @@ func (r *FUSEMknodReq) MarshalUnsafe(buf []byte) {
 	copy(buf[r.MknodIn.SizeBytes():], r.Name)
 }
 
+// MarshalBytes serializes r.MknodIn and r.Name to the dst buffer.
+func (r *FUSEMknodReq) MarshalBytes(buf []byte) {
+	r.MknodIn.MarshalBytes(buf[:r.MknodIn.SizeBytes()])
+	copy(buf[r.MknodIn.SizeBytes():], r.Name)
+}
+
 // SizeBytes is the size of the memory representation of FUSEMknodReq.
 // 1 extra byte for null-terminated string.
 func (r *FUSEMknodReq) SizeBytes() int {
@@ -516,6 +527,13 @@ func (r *FUSESymLinkIn) MarshalUnsafe(buf []byte) {
 	copy(buf[len(r.Name)+1:], r.Target)
 }
 
+// MarshalBytes serializes r.Name and r.Link to the dst buffer.
+// Left null-termination at end of r.Name and r.Link.
+func (r *FUSESymLinkIn) MarshalBytes(buf []byte) {
+	copy(buf, r.Name)
+	copy(buf[len(r.Name)+1:], r.Target)
+}
+
 // SizeBytes is the size of the memory representation of FUSESymLinkIn.
 // 2 extra bytes for null-terminated string.
 func (r *FUSESymLinkIn) SizeBytes() int {
@@ -528,6 +546,9 @@ type FUSEEmptyIn struct{ marshal.StubMarshallable }
 // MarshalUnsafe do nothing for marshal.
 func (r *FUSEEmptyIn) MarshalUnsafe(buf []byte) {}
 
+// MarshalBytes do nothing for marshal.
+func (r *FUSEEmptyIn) MarshalBytes(buf []byte) {}
+
 // SizeBytes is 0 for empty request.
 func (r *FUSEEmptyIn) SizeBytes() int {
 	return 0
@@ -565,6 +586,12 @@ func (r *FUSEMkdirReq) MarshalUnsafe(buf []byte) {
 	copy(buf[r.MkdirIn.SizeBytes():], r.Name)
 }
 
+// MarshalBytes serializes r.MkdirIn and r.Name to the dst buffer.
+func (r *FUSEMkdirReq) MarshalBytes(buf []byte) {
+	r.MkdirIn.MarshalBytes(buf[:r.MkdirIn.SizeBytes()])
+	copy(buf[r.MkdirIn.SizeBytes():], r.Name)
+}
+
 // SizeBytes is the size of the memory representation of FUSEMkdirReq.
 // 1 extra byte for null-terminated Name string.
 func (r *FUSEMkdirReq) SizeBytes() int {
@@ -587,6 +614,11 @@ func (r *FUSERmDirIn) MarshalUnsafe(buf []byte) {
 	copy(buf, r.Name)
 }
 
+// MarshalBytes serializes r.name to the dst buffer.
+func (r *FUSERmDirIn) MarshalBytes(buf []byte) {
+	copy(buf, r.Name)
+}
+
 // SizeBytes is the size of the memory representation of FUSERmDirIn.
 func (r *FUSERmDirIn) SizeBytes() int {
 	return len(r.Name) + 1
@@ -597,6 +629,11 @@ func (r *FUSERmDirIn) UnmarshalUnsafe(src []byte) {
 	r.Name = string(src)
 }
 
+// UnmarshalBytes deserializes r.name from the src buffer.
+func (r *FUSERmDirIn) UnmarshalBytes(src []byte) {
+	r.Name = string(src)
+}
+
 // FUSEDirents is a list of Dirents received from the FUSE daemon server.
 // It is used for FUSE_READDIR.
 //
@@ -647,6 +684,14 @@ func (r *FUSEDirents) MarshalUnsafe(dst []byte) {
 	}
 }
 
+// MarshalBytes serializes FUSEDirents to the dst buffer.
+func (r *FUSEDirents) MarshalBytes(dst []byte) {
+	for _, dirent := range r.Dirents {
+		dirent.MarshalBytes(dst)
+		dst = dst[dirent.SizeBytes():]
+	}
+}
+
 // SizeBytes is the size of the memory representation of FUSEDirents.
 func (r *FUSEDirents) SizeBytes() int {
 	var sizeBytes int
@@ -681,6 +726,30 @@ func (r *FUSEDirents) UnmarshalUnsafe(src []byte) {
 	}
 }
 
+// UnmarshalBytes deserializes FUSEDirents from the src buffer.
+func (r *FUSEDirents) UnmarshalBytes(src []byte) {
+	for {
+		if len(src) <= (*FUSEDirentMeta)(nil).SizeBytes() {
+			break
+		}
+
+		// Its unclear how many dirents there are in src. Each dirent is dynamically
+		// sized and so we can't make assumptions about how many dirents we can allocate.
+		if r.Dirents == nil {
+			r.Dirents = make([]*FUSEDirent, 0)
+		}
+
+		// We have to allocate a struct for each dirent - there must be a better way
+		// to do this. Linux allocates 1 page to store all the dirents and then
+		// simply reads them from the page.
+		var dirent FUSEDirent
+		dirent.UnmarshalBytes(src)
+		r.Dirents = append(r.Dirents, &dirent)
+
+		src = src[dirent.SizeBytes():]
+	}
+}
+
 // MarshalUnsafe serializes FUSEDirent to the dst buffer.
 func (r *FUSEDirent) MarshalUnsafe(dst []byte) {
 	r.Meta.MarshalUnsafe(dst)
@@ -690,6 +759,15 @@ func (r *FUSEDirent) MarshalUnsafe(dst []byte) {
 	name.MarshalUnsafe(dst)
 }
 
+// MarshalBytes serializes FUSEDirent to the dst buffer.
+func (r *FUSEDirent) MarshalBytes(dst []byte) {
+	r.Meta.MarshalBytes(dst)
+	dst = dst[r.Meta.SizeBytes():]
+
+	name := primitive.ByteSlice(r.Name)
+	name.MarshalBytes(dst)
+}
+
 // SizeBytes is the size of the memory representation of FUSEDirent.
 func (r *FUSEDirent) SizeBytes() int {
 	dataSize := r.Meta.SizeBytes() + len(r.Name)
@@ -716,3 +794,20 @@ func (r *FUSEDirent) UnmarshalUnsafe(src []byte) {
 	name.UnmarshalUnsafe(src[:r.Meta.NameLen])
 	r.Name = string(name)
 }
+
+// UnmarshalBytes deserializes FUSEDirent from the src buffer.
+func (r *FUSEDirent) UnmarshalBytes(src []byte) {
+	r.Meta.UnmarshalBytes(src)
+	src = src[r.Meta.SizeBytes():]
+
+	if r.Meta.NameLen > FUSE_NAME_MAX {
+		// The name is too long and therefore invalid. We don't
+		// need to unmarshal the name since it'll be thrown away.
+		return
+	}
+
+	buf := make([]byte, r.Meta.NameLen)
+	name := primitive.ByteSlice(buf)
+	name.UnmarshalBytes(src[:r.Meta.NameLen])
+	r.Name = string(name)
+}

pkg/sentry/fsimpl/fuse/connection.go

@@ -270,8 +270,10 @@ func (conn *connection) NewRequest(creds *auth.Credentials, pid uint32, ino uint
 	}
 
 	buf := make([]byte, hdr.Len)
-	hdr.MarshalUnsafe(buf[:hdrLen])
-	payload.MarshalUnsafe(buf[hdrLen:])
+
+	// TODO(gVisor.dev/3698): Use the unsafe version once go_marshal is safe to use again.
+	hdr.MarshalBytes(buf[:hdrLen])
+	payload.MarshalBytes(buf[hdrLen:])
 
 	return &Request{
 		id:   hdr.Unique,
@@ -359,7 +361,8 @@ func (r *Response) UnmarshalPayload(m marshal.Marshallable) error {
 		return nil
 	}
 
-	m.UnmarshalUnsafe(r.data[hdrLen:])
+	// TODO(gVisor.dev/3698): Use the unsafe version once go_marshal is safe to use again.
+	m.UnmarshalBytes(r.data[hdrLen:])
 	return nil
 }
 

tools/go_marshal/marshal/marshal_impl_util.go

@@ -44,7 +44,7 @@ func (StubMarshallable) MarshalBytes(dst []byte) {
 
 // UnmarshalBytes implements Marshallable.UnmarshalBytes.
 func (StubMarshallable) UnmarshalBytes(src []byte) {
-	panic("Please implement your own UnMarshalBytes function")
+	panic("Please implement your own UnmarshalBytes function")
 }
 
 // Packed implements Marshallable.Packed.