diff --git a/pkg/abi/linux/fuse.go b/pkg/abi/linux/fuse.go index a7e028237..5291f572e 100644 --- a/pkg/abi/linux/fuse.go +++ b/pkg/abi/linux/fuse.go @@ -113,6 +113,9 @@ type FUSEHeaderIn struct { _ uint32 } +// SizeOfFUSEHeaderIn is the size of the FUSEHeaderIn struct. +var SizeOfFUSEHeaderIn = uint32((*FUSEHeaderIn)(nil).SizeBytes()) + // FUSEHeaderOut is the header written by the daemon when it processes // a request and wants to send a reply (almost all operations require a // reply; if they do not, this will be explicitly documented). @@ -129,6 +132,9 @@ type FUSEHeaderOut struct { Unique FUSEOpID } +// SizeOfFUSEHeaderOut is the size of the FUSEHeaderOut struct. +var SizeOfFUSEHeaderOut = uint32((*FUSEHeaderOut)(nil).SizeBytes()) + // FUSE_INIT flags, consistent with the ones in include/uapi/linux/fuse.h. // Our taget version is 7.23 but we have few implemented in advance. const ( @@ -442,7 +448,7 @@ const MAX_NON_LFS = ((1 << 31) - 1) const ( // FOPEN_DIRECT_IO indicates bypassing page cache for this opened file. FOPEN_DIRECT_IO = 1 << 0 - // FOPEN_KEEP_CACHE avoids invalidate of data cache on open. + // FOPEN_KEEP_CACHE avoids invalidating the data cache on open. FOPEN_KEEP_CACHE = 1 << 1 // FOPEN_NONSEEKABLE indicates the file cannot be seeked. FOPEN_NONSEEKABLE = 1 << 2 @@ -464,10 +470,10 @@ type FUSEOpenIn struct { // // +marshal type FUSEOpenOut struct { - // Fh is the file handler for opened file. + // Fh is the file handler for opened files. Fh uint64 - // OpenFlag for the opened file. + // OpenFlag for the opened files. OpenFlag uint32 _ uint32 @@ -545,6 +551,9 @@ type FUSEWriteIn struct { _ uint32 } +// SizeOfFUSEWriteIn is the size of the FUSEWriteIn struct. +var SizeOfFUSEWriteIn = uint32((*FUSEWriteIn)(nil).SizeBytes()) + // FUSEWritePayloadIn combines header - FUSEWriteIn and payload // in a single marshallable struct when sending request by the // kernel to the daemon @@ -651,7 +660,7 @@ func (r *FUSERenameIn) SizeBytes() int { // // +marshal dynamic type FUSECreateIn struct { - // CreateMeta contains mode, rdev and umash field for FUSE_MKNODS. + // CreateMeta contains mode, rdev and umash fields for FUSE_MKNODS. CreateMeta FUSECreateMeta // Name is the name of the node to create. @@ -696,7 +705,7 @@ type FUSEMknodMeta struct { // // +marshal dynamic type FUSEMknodIn struct { - // MknodMeta contains mode, rdev and umash field for FUSE_MKNODS. + // MknodMeta contains mode, rdev and umash fields for FUSE_MKNODS. MknodMeta FUSEMknodMeta // Name is the name of the node to create. Name CString diff --git a/pkg/sentry/fsimpl/fuse/connection.go b/pkg/sentry/fsimpl/fuse/connection.go index b9990afa6..3ad7e9624 100644 --- a/pkg/sentry/fsimpl/fuse/connection.go +++ b/pkg/sentry/fsimpl/fuse/connection.go @@ -210,11 +210,8 @@ func newFUSEConnection(_ context.Context, fuseFD *DeviceFD, opts *filesystemOpti // mount another filesystem. // Create the writeBuf for the header to be stored in. - hdrLen := uint32((*linux.FUSEHeaderOut)(nil).SizeBytes()) - fuseFD.writeBuf = make([]byte, hdrLen) fuseFD.completions = make(map[linux.FUSEOpID]*futureResponse) fuseFD.fullQueueCh = make(chan struct{}, opts.maxActiveRequests) - fuseFD.writeCursor = 0 return &connection{ fd: fuseFD, @@ -255,7 +252,7 @@ func (conn *connection) CallAsync(t *kernel.Task, r *Request) error { // The forget request does not have a reply, // as documented in include/uapi/linux/fuse.h:FUSE_FORGET. func (conn *connection) Call(t *kernel.Task, r *Request) (*Response, error) { - // Block requests sent before connection is initalized. + // Block requests sent before connection is initialized. if !conn.Initialized() && r.hdr.Opcode != linux.FUSE_INIT { if err := t.Block(conn.initializedChan); err != nil { return nil, err diff --git a/pkg/sentry/fsimpl/fuse/dev.go b/pkg/sentry/fsimpl/fuse/dev.go index d24d843c1..5ce222c26 100644 --- a/pkg/sentry/fsimpl/fuse/dev.go +++ b/pkg/sentry/fsimpl/fuse/dev.go @@ -29,6 +29,9 @@ import ( const fuseDevMinor = 229 +// This is equivalent to linux.SizeOfFUSEHeaderIn +const fuseHeaderOutSize = 16 + // fuseDevice implements vfs.Device for /dev/fuse. // // +stateify savable @@ -84,17 +87,10 @@ type DeviceFD struct { // +checklocks:mu completions map[linux.FUSEOpID]*futureResponse - // +checklocks:mu - writeCursor uint32 - // writeBuf is the memory buffer used to copy in the FUSE out header from // userspace. // +checklocks:mu - writeBuf []byte - - // writeCursorFR current FR being copied from server. - // +checklocks:mu - writeCursorFR *futureResponse + writeBuf [fuseHeaderOutSize]byte // conn is the FUSE connection that this FD is being used for. // +checklocks:mu @@ -144,9 +140,6 @@ func (fd *DeviceFD) PRead(ctx context.Context, dst usermem.IOSequence, offset in // Read implements vfs.FileDescriptionImpl.Read. func (fd *DeviceFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) { - // Operations on /dev/fuse don't make sense until a FUSE filesystem is - // mounted. If there is an active connection we know there is at least one - // filesystem mounted. fd.mu.Lock() defer fd.mu.Unlock() if !fd.connected() { @@ -158,11 +151,8 @@ func (fd *DeviceFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.R // header (Linux uses the request header and the FUSEWriteIn header for this // calculation) + the negotiated MaxWrite room for the data. minBuffSize := linux.FUSE_MIN_READ_BUFFER - inHdrLen := uint32((*linux.FUSEHeaderIn)(nil).SizeBytes()) - writeHdrLen := uint32((*linux.FUSEWriteIn)(nil).SizeBytes()) - fd.conn.mu.Lock() - negotiatedMinBuffSize := inHdrLen + writeHdrLen + fd.conn.maxWrite + negotiatedMinBuffSize := linux.SizeOfFUSEHeaderIn + linux.SizeOfFUSEHeaderOut + fd.conn.maxWrite fd.conn.mu.Unlock() if minBuffSize < negotiatedMinBuffSize { minBuffSize = negotiatedMinBuffSize @@ -180,44 +170,32 @@ func (fd *DeviceFD) Read(ctx context.Context, dst usermem.IOSequence, opts vfs.R // Preconditions: dst is large enough for any reasonable request. // +checklocks:fd.mu func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts vfs.ReadOptions) (int64, error) { + // Find the first valid request. For the normal case this loop only executes + // once. var req *Request - - // Find the first valid request. - // For the normal case this loop only execute once. - for !fd.queue.Empty() { - req = fd.queue.Front() - + for req = fd.queue.Front(); !fd.queue.Empty(); req = fd.queue.Front() { if int64(req.hdr.Len) <= dst.NumBytes() { break } - - // The request is too large. Cannot process it. All requests must be smaller than the - // negotiated size as specified by Connection.MaxWrite set as part of the FUSE_INIT - // handshake. + // The request is too large so we cannot process it. All requests must be + // smaller than the negotiated size as specified by Connection.MaxWrite set + // as part of the FUSE_INIT handshake. errno := -int32(unix.EIO) if req.hdr.Opcode == linux.FUSE_SETXATTR { errno = -int32(unix.E2BIG) } - // Return the error to the calling task. if err := fd.sendError(ctx, errno, req.hdr.Unique); err != nil { return 0, err } - - // We're done with this request. fd.queue.Remove(req) req = nil } - if req == nil { return 0, linuxerr.ErrWouldBlock } // We already checked the size: dst must be able to fit the whole request. - // Now we write the marshalled header, the payload, - // and the potential additional payload - // to the user memory IOSequence. - n, err := dst.CopyOut(ctx, req.data) if err != nil { return 0, err @@ -225,13 +203,10 @@ func (fd *DeviceFD) readLocked(ctx context.Context, dst usermem.IOSequence, opts if n != len(req.data) { return 0, linuxerr.EIO } - - // Fully done with this req, remove it from the queue. fd.queue.Remove(req) - - // Remove noReply ones from map of requests expecting a reply. + // Remove noReply ones from the map of requests expecting a reply. if req.noReply { - fd.numActiveRequests -= 1 + fd.numActiveRequests-- delete(fd.completions, req.hdr.Unique) } @@ -262,102 +237,36 @@ func (fd *DeviceFD) Write(ctx context.Context, src usermem.IOSequence, opts vfs. // writeLocked implements writing to the fuse device while locked with DeviceFD.mu. // +checklocks:fd.mu func (fd *DeviceFD) writeLocked(ctx context.Context, src usermem.IOSequence, opts vfs.WriteOptions) (int64, error) { - // Operations on /dev/fuse don't make sense until a FUSE filesystem is - // mounted. If there is an active connection we know there is at least one - // filesystem mounted. if !fd.connected() { return 0, linuxerr.EPERM } - var cn, n int64 - hdrLen := uint32((*linux.FUSEHeaderOut)(nil).SizeBytes()) - - for src.NumBytes() > 0 { - if fd.writeCursorFR != nil { - // Already have common header, and we're now copying the payload. - wantBytes := fd.writeCursorFR.hdr.Len - - // Note that the FR data doesn't have the header. Copy it over if its necessary. - if fd.writeCursorFR.data == nil { - fd.writeCursorFR.data = make([]byte, wantBytes) - } - - bytesCopied, err := src.CopyIn(ctx, fd.writeCursorFR.data[fd.writeCursor:wantBytes]) - if err != nil { - return 0, err - } - src = src.DropFirst(bytesCopied) - - cn = int64(bytesCopied) - n += cn - fd.writeCursor += uint32(cn) - if fd.writeCursor == wantBytes { - // Done reading this full response. Clean up and unblock the - // initiator. - break - } - - // Check if we have more data in src. - continue - } - - // Assert that the header isn't read into the writeBuf yet. - if fd.writeCursor >= hdrLen { - return 0, linuxerr.EINVAL - } - - // We don't have the full common response header yet. - wantBytes := hdrLen - fd.writeCursor - bytesCopied, err := src.CopyIn(ctx, fd.writeBuf[fd.writeCursor:wantBytes]) - if err != nil { - return 0, err - } - src = src.DropFirst(bytesCopied) - - cn = int64(bytesCopied) - n += cn - fd.writeCursor += uint32(cn) - if fd.writeCursor == hdrLen { - // Have full header in the writeBuf. Use it to fetch the actual futureResponse - // from the device's completions map. - var hdr linux.FUSEHeaderOut - hdr.UnmarshalBytes(fd.writeBuf) - - // We have the header now and so the writeBuf has served its purpose. - // We could reset it manually here but instead of doing that, at the - // end of the write, the writeCursor will be set to 0 thereby allowing - // the next request to overwrite whats in the buffer, - - fut, ok := fd.completions[hdr.Unique] - if !ok { - // Server sent us a response for a request we never sent, - // or for which we already received a reply (e.g. aborted), an unlikely event. - return 0, linuxerr.EINVAL - } - - delete(fd.completions, hdr.Unique) - - // Copy over the header into the future response. The rest of the payload - // will be copied over to the FR's data in the next iteration. - fut.hdr = &hdr - fd.writeCursorFR = fut - - // Next iteration will now try read the complete request, if src has - // any data remaining. Otherwise we're done. - } + if _, err := src.CopyIn(ctx, fd.writeBuf[:]); err != nil { + return 0, err } + var hdr linux.FUSEHeaderOut + hdr.UnmarshalBytes(fd.writeBuf[:]) - if fd.writeCursorFR != nil { - if err := fd.sendResponse(ctx, fd.writeCursorFR); err != nil { - return 0, err - } - - // Ready the device for the next request. - fd.writeCursorFR = nil - fd.writeCursor = 0 + fut, ok := fd.completions[hdr.Unique] + if !ok { + // Server sent us a response for a request we never sent, or for which we + // already received a reply (e.g. aborted), an unlikely event. + return 0, linuxerr.EINVAL } + delete(fd.completions, hdr.Unique) - return n, nil + // Copy over the header into the future response. The rest of the payload + // will be copied over to the FR's data in the next iteration. + fut.hdr = &hdr + fut.data = make([]byte, fut.hdr.Len) + n, err := src.CopyIn(ctx, fut.data) + if err != nil { + return 0, err + } + if err := fd.sendResponse(ctx, fut); err != nil { + return 0, err + } + return int64(n), nil } // Readiness implements vfs.FileDescriptionImpl.Readiness. @@ -448,9 +357,8 @@ func (fd *DeviceFD) sendResponse(ctx context.Context, fut *futureResponse) error // +checklocks:fd.mu func (fd *DeviceFD) sendError(ctx context.Context, errno int32, unique linux.FUSEOpID) error { // Return the error to the calling task. - outHdrLen := uint32((*linux.FUSEHeaderOut)(nil).SizeBytes()) respHdr := linux.FUSEHeaderOut{ - Len: outHdrLen, + Len: linux.SizeOfFUSEHeaderOut, Error: errno, Unique: unique, } diff --git a/pkg/sentry/fsimpl/fuse/dev_test.go b/pkg/sentry/fsimpl/fuse/dev_test.go index 91b01fe1c..736ea8c4e 100644 --- a/pkg/sentry/fsimpl/fuse/dev_test.go +++ b/pkg/sentry/fsimpl/fuse/dev_test.go @@ -295,11 +295,10 @@ func fuseServerRun(t *testing.T, s *testutil.System, k *kernel.Kernel, fd *vfs.F // Read the request. for { - inHdrLen := uint32((*linux.FUSEHeaderIn)(nil).SizeBytes()) payloadLen := uint32(readPayload.SizeBytes()) - // The raed buffer must meet some certain size criteria. - buffSize := inHdrLen + payloadLen + // The read buffer must meet some certain size criteria. + buffSize := linux.SizeOfFUSEHeaderIn + payloadLen if buffSize < linux.FUSE_MIN_READ_BUFFER { buffSize = linux.FUSE_MIN_READ_BUFFER } @@ -311,7 +310,7 @@ func fuseServerRun(t *testing.T, s *testutil.System, k *kernel.Kernel, fd *vfs.F t.Fatalf("Read failed :%v", err) } - // Server should shut down. No new requests are going to be made. + // The server should shut down. No new requests are going to be made. if serverKilled { break } @@ -329,17 +328,16 @@ func fuseServerRun(t *testing.T, s *testutil.System, k *kernel.Kernel, fd *vfs.F } // Write the response. - outHdrLen := uint32((*linux.FUSEHeaderOut)(nil).SizeBytes()) - outBuf := make([]byte, outHdrLen+payloadLen) + outBuf := make([]byte, linux.SizeOfFUSEHeaderOut+payloadLen) outHeader := linux.FUSEHeaderOut{ - Len: outHdrLen + payloadLen, + Len: linux.SizeOfFUSEHeaderOut + payloadLen, Error: 0, Unique: readFUSEHeaderIn.Unique, } // Echo the payload back. - outHeader.MarshalUnsafe(outBuf[:outHdrLen]) - readPayload.MarshalUnsafe(outBuf[outHdrLen:]) + outHeader.MarshalUnsafe(outBuf[:linux.SizeOfFUSEHeaderOut]) + readPayload.MarshalUnsafe(outBuf[linux.SizeOfFUSEHeaderOut:]) outIOseq := usermem.BytesIOSequence(outBuf) _, err = fd.Write(s.Ctx, outIOseq, vfs.WriteOptions{})