diff options
-rw-r--r-- | pkg/sentry/fs/ext/block_map_file.go | 82 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/block_map_test.go | 14 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/extent_file.go | 91 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/extent_test.go | 17 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/filesystem.go | 13 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/inline_file.go | 35 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/inode.go | 24 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/regular_file.go | 22 | ||||
-rw-r--r-- | pkg/sentry/fs/ext/symlink.go | 9 |
9 files changed, 121 insertions, 186 deletions
diff --git a/pkg/sentry/fs/ext/block_map_file.go b/pkg/sentry/fs/ext/block_map_file.go index e9b11d65d..f30c3a174 100644 --- a/pkg/sentry/fs/ext/block_map_file.go +++ b/pkg/sentry/fs/ext/block_map_file.go @@ -53,27 +53,17 @@ type blockMapFile struct { coverage [4]uint64 } -// Compiles only if blockMapFile implements fileReader. -var _ fileReader = (*blockMapFile)(nil) - -// Read implements fileReader.getFileReader. -func (f *blockMapFile) getFileReader(dev io.ReaderAt, blkSize uint64, offset uint64) io.Reader { - return &blockMapReader{ - dev: dev, - file: f, - fileOff: offset, - blkSize: blkSize, - } -} +// Compiles only if blockMapFile implements io.ReaderAt. +var _ io.ReaderAt = (*blockMapFile)(nil) // newBlockMapFile is the blockMapFile constructor. It initializes the file to // physical blocks map with (at most) the first 12 (direct) blocks. -func newBlockMapFile(blkSize uint64, regFile regularFile) (*blockMapFile, error) { +func newBlockMapFile(regFile regularFile) (*blockMapFile, error) { file := &blockMapFile{regFile: regFile} file.regFile.impl = file for i := uint(0); i < 4; i++ { - file.coverage[i] = getCoverage(blkSize, i) + file.coverage[i] = getCoverage(regFile.inode.blkSize, i) } blkMap := regFile.inode.diskInode.Data() @@ -84,40 +74,33 @@ func newBlockMapFile(blkSize uint64, regFile regularFile) (*blockMapFile, error) return file, nil } -// blockMapReader implements io.Reader which will fetch fill data from the -// block maps and build the blockMapFile.fileToPhyBlks array if required. -type blockMapReader struct { - dev io.ReaderAt - file *blockMapFile - fileOff uint64 - blkSize uint64 -} - -// Compiles only if blockMapReader implements io.Reader. -var _ io.Reader = (*blockMapReader)(nil) - -// Read implements io.Reader.Read. -func (r *blockMapReader) Read(dst []byte) (int, error) { +// ReadAt implements io.ReaderAt.ReadAt. +func (f *blockMapFile) ReadAt(dst []byte, off int64) (int, error) { if len(dst) == 0 { return 0, nil } - if r.fileOff >= r.file.regFile.inode.diskInode.Size() { + if off < 0 { + return 0, syserror.EINVAL + } + + offset := uint64(off) + if offset >= f.regFile.inode.diskInode.Size() { return 0, io.EOF } // dirBlksEnd is the file offset until which direct blocks cover file data. // Direct blocks cover 0 <= file offset < dirBlksEnd. - dirBlksEnd := numDirectBlks * r.file.coverage[0] + dirBlksEnd := numDirectBlks * f.coverage[0] // indirBlkEnd is the file offset until which the indirect block covers file // data. The indirect block covers dirBlksEnd <= file offset < indirBlkEnd. - indirBlkEnd := dirBlksEnd + r.file.coverage[1] + indirBlkEnd := dirBlksEnd + f.coverage[1] // doubIndirBlkEnd is the file offset until which the double indirect block // covers file data. The double indirect block covers the range // indirBlkEnd <= file offset < doubIndirBlkEnd. - doubIndirBlkEnd := indirBlkEnd + r.file.coverage[2] + doubIndirBlkEnd := indirBlkEnd + f.coverage[2] read := 0 toRead := len(dst) @@ -127,21 +110,22 @@ func (r *blockMapReader) Read(dst []byte) (int, error) { // Figure out which block to delegate the read to. switch { - case r.fileOff < dirBlksEnd: + case offset < dirBlksEnd: // Direct block. - curR, err = r.read(r.file.directBlks[r.fileOff/r.blkSize], r.fileOff%r.blkSize, 0, dst[read:]) - case r.fileOff < indirBlkEnd: + curR, err = f.read(f.directBlks[offset/f.regFile.inode.blkSize], offset%f.regFile.inode.blkSize, 0, dst[read:]) + case offset < indirBlkEnd: // Indirect block. - curR, err = r.read(r.file.indirectBlk, r.fileOff-dirBlksEnd, 1, dst[read:]) - case r.fileOff < doubIndirBlkEnd: + curR, err = f.read(f.indirectBlk, offset-dirBlksEnd, 1, dst[read:]) + case offset < doubIndirBlkEnd: // Doubly indirect block. - curR, err = r.read(r.file.doubleIndirectBlk, r.fileOff-indirBlkEnd, 2, dst[read:]) + curR, err = f.read(f.doubleIndirectBlk, offset-indirBlkEnd, 2, dst[read:]) default: // Triply indirect block. - curR, err = r.read(r.file.tripleIndirectBlk, r.fileOff-doubIndirBlkEnd, 3, dst[read:]) + curR, err = f.read(f.tripleIndirectBlk, offset-doubIndirBlkEnd, 3, dst[read:]) } read += curR + offset += uint64(curR) if err != nil { return read, err } @@ -150,30 +134,29 @@ func (r *blockMapReader) Read(dst []byte) (int, error) { return read, nil } -// read is the recursive step of the Read function. It relies on knowing the +// read is the recursive step of the ReadAt function. It relies on knowing the // current node's location on disk (curPhyBlk) and its height in the block map // tree. A height of 0 shows that the current node is actually holding file // data. relFileOff tells the offset from which we need to start to reading // under the current node. It is completely relative to the current node. -func (r *blockMapReader) read(curPhyBlk uint32, relFileOff uint64, height uint, dst []byte) (int, error) { - curPhyBlkOff := int64(curPhyBlk) * int64(r.blkSize) +func (f *blockMapFile) read(curPhyBlk uint32, relFileOff uint64, height uint, dst []byte) (int, error) { + curPhyBlkOff := int64(curPhyBlk) * int64(f.regFile.inode.blkSize) if height == 0 { - toRead := int(r.blkSize - relFileOff) + toRead := int(f.regFile.inode.blkSize - relFileOff) if len(dst) < toRead { toRead = len(dst) } - n, _ := r.dev.ReadAt(dst[:toRead], curPhyBlkOff+int64(relFileOff)) - r.fileOff += uint64(n) + n, _ := f.regFile.inode.dev.ReadAt(dst[:toRead], curPhyBlkOff+int64(relFileOff)) if n < toRead { return n, syserror.EIO } return n, nil } - childCov := r.file.coverage[height-1] + childCov := f.coverage[height-1] startIdx := relFileOff / childCov - endIdx := r.blkSize / 4 // This is exclusive. + endIdx := f.regFile.inode.blkSize / 4 // This is exclusive. wantEndIdx := (relFileOff + uint64(len(dst))) / childCov wantEndIdx++ // Make this exclusive. if wantEndIdx < endIdx { @@ -184,11 +167,12 @@ func (r *blockMapReader) read(curPhyBlk uint32, relFileOff uint64, height uint, curChildOff := relFileOff % childCov for i := startIdx; i < endIdx; i++ { var childPhyBlk uint32 - if err := readFromDisk(r.dev, curPhyBlkOff+int64(i*4), &childPhyBlk); err != nil { + err := readFromDisk(f.regFile.inode.dev, curPhyBlkOff+int64(i*4), &childPhyBlk) + if err != nil { return read, err } - n, err := r.read(childPhyBlk, curChildOff, height-1, dst[read:]) + n, err := f.read(childPhyBlk, curChildOff, height-1, dst[read:]) read += n if err != nil { return read, err diff --git a/pkg/sentry/fs/ext/block_map_test.go b/pkg/sentry/fs/ext/block_map_test.go index b92ae80c8..f8dd6bf9f 100644 --- a/pkg/sentry/fs/ext/block_map_test.go +++ b/pkg/sentry/fs/ext/block_map_test.go @@ -16,7 +16,6 @@ package ext import ( "bytes" - "io" "math/rand" "testing" @@ -34,14 +33,13 @@ const ( // TestBlockMapReader stress tests block map reader functionality. It performs // random length reads from all possible positions in the block map structure. func TestBlockMapReader(t *testing.T) { - dev, mockBMFile, want := blockMapSetUp(t) + mockBMFile, want := blockMapSetUp(t) n := len(want) for from := 0; from < n; from++ { - fileReader := mockBMFile.getFileReader(dev, uint64(mockBMBlkSize), uint64(from)) got := make([]byte, n-from) - if read, err := io.ReadFull(fileReader, got); err != nil { + if read, err := mockBMFile.ReadAt(got, int64(from)); err != nil { t.Fatalf("file read operation from offset %d to %d only read %d bytes: %v", from, n, read, err) } @@ -85,7 +83,7 @@ func (n *blkNumGen) next() uint32 { // block and 1 triple indirect block (basically fill it till the rim). It // initializes the disk to reflect the inode. Also returns the file data that // the inode covers and that is written to disk. -func blockMapSetUp(t *testing.T) (io.ReaderAt, *blockMapFile, []byte) { +func blockMapSetUp(t *testing.T) (*blockMapFile, []byte) { mockDisk := make([]byte, mockBMDiskSize) regFile := regularFile{ inode: inode{ @@ -94,6 +92,8 @@ func blockMapSetUp(t *testing.T) (io.ReaderAt, *blockMapFile, []byte) { SizeLo: getMockBMFileFize(), }, }, + dev: bytes.NewReader(mockDisk), + blkSize: uint64(mockBMBlkSize), }, } @@ -125,11 +125,11 @@ func blockMapSetUp(t *testing.T) (io.ReaderAt, *blockMapFile, []byte) { copy(regFile.inode.diskInode.Data(), data) - mockFile, err := newBlockMapFile(uint64(mockBMBlkSize), regFile) + mockFile, err := newBlockMapFile(regFile) if err != nil { t.Fatalf("newBlockMapFile failed: %v", err) } - return bytes.NewReader(mockDisk), mockFile, fileData + return mockFile, fileData } // writeFileDataToBlock writes random bytes to the block on disk. diff --git a/pkg/sentry/fs/ext/extent_file.go b/pkg/sentry/fs/ext/extent_file.go index aa4102dbb..44fb9c01f 100644 --- a/pkg/sentry/fs/ext/extent_file.go +++ b/pkg/sentry/fs/ext/extent_file.go @@ -32,26 +32,16 @@ type extentFile struct { root disklayout.ExtentNode } -// Compiles only if extentFile implements fileReader. -var _ fileReader = (*extentFile)(nil) - -// Read implements fileReader.getFileReader. -func (f *extentFile) getFileReader(dev io.ReaderAt, blkSize uint64, offset uint64) io.Reader { - return &extentReader{ - dev: dev, - file: f, - fileOff: offset, - blkSize: blkSize, - } -} +// Compiles only if extentFile implements io.ReaderAt. +var _ io.ReaderAt = (*extentFile)(nil) // newExtentFile is the extent file constructor. It reads the entire extent // tree into memory. // TODO(b/134676337): Build extent tree on demand to reduce memory usage. -func newExtentFile(dev io.ReaderAt, blkSize uint64, regFile regularFile) (*extentFile, error) { +func newExtentFile(regFile regularFile) (*extentFile, error) { file := &extentFile{regFile: regFile} file.regFile.impl = file - err := file.buildExtTree(dev, blkSize) + err := file.buildExtTree() if err != nil { return nil, err } @@ -64,7 +54,7 @@ func newExtentFile(dev io.ReaderAt, blkSize uint64, regFile regularFile) (*exten // disk. // // Precondition: inode flag InExtents must be set. -func (f *extentFile) buildExtTree(dev io.ReaderAt, blkSize uint64) error { +func (f *extentFile) buildExtTree() error { rootNodeData := f.regFile.inode.diskInode.Data() binary.Unmarshal(rootNodeData[:disklayout.ExtentStructsSize], binary.LittleEndian, &f.root.Header) @@ -94,7 +84,7 @@ func (f *extentFile) buildExtTree(dev io.ReaderAt, blkSize uint64) error { if f.root.Header.Height > 0 { for i := uint16(0); i < f.root.Header.NumEntries; i++ { var err error - if f.root.Entries[i].Node, err = buildExtTreeFromDisk(dev, f.root.Entries[i].Entry, blkSize); err != nil { + if f.root.Entries[i].Node, err = f.buildExtTreeFromDisk(f.root.Entries[i].Entry); err != nil { return err } } @@ -106,10 +96,10 @@ func (f *extentFile) buildExtTree(dev io.ReaderAt, blkSize uint64) error { // buildExtTreeFromDisk reads the extent tree nodes from disk and recursively // builds the tree. Performs a simple DFS. It returns the ExtentNode pointed to // by the ExtentEntry. -func buildExtTreeFromDisk(dev io.ReaderAt, entry disklayout.ExtentEntry, blkSize uint64) (*disklayout.ExtentNode, error) { +func (f *extentFile) buildExtTreeFromDisk(entry disklayout.ExtentEntry) (*disklayout.ExtentNode, error) { var header disklayout.ExtentHeader - off := entry.PhysicalBlock() * blkSize - err := readFromDisk(dev, int64(off), &header) + off := entry.PhysicalBlock() * f.regFile.inode.blkSize + err := readFromDisk(f.regFile.inode.dev, int64(off), &header) if err != nil { return nil, err } @@ -125,7 +115,7 @@ func buildExtTreeFromDisk(dev io.ReaderAt, entry disklayout.ExtentEntry, blkSize curEntry = &disklayout.ExtentIdx{} } - err := readFromDisk(dev, int64(off), curEntry) + err := readFromDisk(f.regFile.inode.dev, int64(off), curEntry) if err != nil { return nil, err } @@ -136,7 +126,7 @@ func buildExtTreeFromDisk(dev io.ReaderAt, entry disklayout.ExtentEntry, blkSize if header.Height > 0 { for i := uint16(0); i < header.NumEntries; i++ { var err error - entries[i].Node, err = buildExtTreeFromDisk(dev, entries[i].Entry, blkSize) + entries[i].Node, err = f.buildExtTreeFromDisk(entries[i].Entry) if err != nil { return nil, err } @@ -146,38 +136,31 @@ func buildExtTreeFromDisk(dev io.ReaderAt, entry disklayout.ExtentEntry, blkSize return &disklayout.ExtentNode{header, entries}, nil } -// extentReader implements io.Reader which can traverse the extent tree and -// read file data. This is not thread safe. -type extentReader struct { - dev io.ReaderAt - file *extentFile - fileOff uint64 // Represents the current file offset being read from. - blkSize uint64 -} - -// Compiles only if inlineReader implements io.Reader. -var _ io.Reader = (*extentReader)(nil) - -// Read implements io.Reader.Read. -func (r *extentReader) Read(dst []byte) (int, error) { +// ReadAt implements io.ReaderAt.ReadAt. +func (f *extentFile) ReadAt(dst []byte, off int64) (int, error) { if len(dst) == 0 { return 0, nil } - if r.fileOff >= r.file.regFile.inode.diskInode.Size() { + if off < 0 { + return 0, syserror.EINVAL + } + + if uint64(off) >= f.regFile.inode.diskInode.Size() { return 0, io.EOF } - return r.read(&r.file.root, dst) + return f.read(&f.root, uint64(off), dst) } -// read is a helper which traverses the extent tree and reads data. -func (r *extentReader) read(node *disklayout.ExtentNode, dst []byte) (int, error) { +// read is the recursive step of extentFile.ReadAt which traverses the extent +// tree from the node passed and reads file data. +func (f *extentFile) read(node *disklayout.ExtentNode, off uint64, dst []byte) (int, error) { // Perform a binary search for the node covering bytes starting at r.fileOff. // A highly fragmented filesystem can have upto 340 entries and so linear // search should be avoided. Finds the first entry which does not cover the // file block we want and subtracts 1 to get the desired index. - fileBlk := r.fileBlock() + fileBlk := uint32(off / f.regFile.inode.blkSize) n := len(node.Entries) found := sort.Search(n, func(i int) bool { return node.Entries[i].Entry.FileBlock() > fileBlk @@ -195,12 +178,13 @@ func (r *extentReader) read(node *disklayout.ExtentNode, dst []byte) (int, error var err error for i := found; i < n && read < toRead; i++ { if node.Header.Height == 0 { - curR, err = r.readFromExtent(node.Entries[i].Entry.(*disklayout.Extent), dst[read:]) + curR, err = f.readFromExtent(node.Entries[i].Entry.(*disklayout.Extent), off, dst[read:]) } else { - curR, err = r.read(node.Entries[i].Node, dst[read:]) + curR, err = f.read(node.Entries[i].Node, off, dst[read:]) } read += curR + off += uint64(curR) if err != nil { return read, err } @@ -211,7 +195,7 @@ func (r *extentReader) read(node *disklayout.ExtentNode, dst []byte) (int, error // readFromExtent reads file data from the extent. It takes advantage of the // sequential nature of extents and reads file data from multiple blocks in one -// call. Also updates the file offset. +// call. // // A non-nil error indicates that this is a partial read and there is probably // more to read from this extent. The caller should propagate the error upward @@ -219,8 +203,8 @@ func (r *extentReader) read(node *disklayout.ExtentNode, dst []byte) (int, error // // A subsequent call to extentReader.Read should continue reading from where we // left off as expected. -func (r *extentReader) readFromExtent(ex *disklayout.Extent, dst []byte) (int, error) { - curFileBlk := r.fileBlock() +func (f *extentFile) readFromExtent(ex *disklayout.Extent, off uint64, dst []byte) (int, error) { + curFileBlk := uint32(off / f.regFile.inode.blkSize) exFirstFileBlk := ex.FileBlock() exLastFileBlk := exFirstFileBlk + uint32(ex.Length) // This is exclusive. @@ -231,30 +215,19 @@ func (r *extentReader) readFromExtent(ex *disklayout.Extent, dst []byte) (int, e } curPhyBlk := uint64(curFileBlk-exFirstFileBlk) + ex.PhysicalBlock() - readStart := curPhyBlk*r.blkSize + r.fileBlockOff() + readStart := curPhyBlk*f.regFile.inode.blkSize + (off % f.regFile.inode.blkSize) endPhyBlk := ex.PhysicalBlock() + uint64(ex.Length) - extentEnd := endPhyBlk * r.blkSize // This is exclusive. + extentEnd := endPhyBlk * f.regFile.inode.blkSize // This is exclusive. toRead := int(extentEnd - readStart) if len(dst) < toRead { toRead = len(dst) } - n, _ := r.dev.ReadAt(dst[:toRead], int64(readStart)) - r.fileOff += uint64(n) + n, _ := f.regFile.inode.dev.ReadAt(dst[:toRead], int64(readStart)) if n < toRead { return n, syserror.EIO } return n, nil } - -// fileBlock returns the file block number we are currently reading. -func (r *extentReader) fileBlock() uint32 { - return uint32(r.fileOff / r.blkSize) -} - -// fileBlockOff returns the current offset within the current file block. -func (r *extentReader) fileBlockOff() uint64 { - return r.fileOff % r.blkSize -} diff --git a/pkg/sentry/fs/ext/extent_test.go b/pkg/sentry/fs/ext/extent_test.go index 9b3f5469e..d03cd564f 100644 --- a/pkg/sentry/fs/ext/extent_test.go +++ b/pkg/sentry/fs/ext/extent_test.go @@ -16,7 +16,6 @@ package ext import ( "bytes" - "io" "math/rand" "testing" @@ -145,14 +144,13 @@ var ( // TestExtentReader stress tests extentReader functionality. It performs random // length reads from all possible positions in the extent tree. func TestExtentReader(t *testing.T) { - dev, mockExtentFile, want := extentTreeSetUp(t, node0) + mockExtentFile, want := extentTreeSetUp(t, node0) n := len(want) for from := 0; from < n; from++ { - fileReader := mockExtentFile.getFileReader(dev, mockExtentBlkSize, uint64(from)) got := make([]byte, n-from) - if read, err := io.ReadFull(fileReader, got); err != nil { + if read, err := mockExtentFile.ReadAt(got, int64(from)); err != nil { t.Fatalf("file read operation from offset %d to %d only read %d bytes: %v", from, n, read, err) } @@ -164,7 +162,7 @@ func TestExtentReader(t *testing.T) { // TestBuildExtentTree tests the extent tree building logic. func TestBuildExtentTree(t *testing.T) { - _, mockExtentFile, _ := extentTreeSetUp(t, node0) + mockExtentFile, _ := extentTreeSetUp(t, node0) opt := cmpopts.IgnoreUnexported(disklayout.ExtentIdx{}, disklayout.ExtentHeader{}) if diff := cmp.Diff(&mockExtentFile.root, node0, opt); diff != "" { @@ -175,7 +173,7 @@ func TestBuildExtentTree(t *testing.T) { // extentTreeSetUp writes the passed extent tree to a mock disk as an extent // tree. It also constucts a mock extent file with the same tree built in it. // It also writes random data file data and returns it. -func extentTreeSetUp(t *testing.T, root *disklayout.ExtentNode) (io.ReaderAt, *extentFile, []byte) { +func extentTreeSetUp(t *testing.T, root *disklayout.ExtentNode) (*extentFile, []byte) { t.Helper() mockDisk := make([]byte, mockExtentBlkSize*10) @@ -187,17 +185,18 @@ func extentTreeSetUp(t *testing.T, root *disklayout.ExtentNode) (io.ReaderAt, *e SizeLo: uint32(mockExtentBlkSize) * getNumPhyBlks(root), }, }, + blkSize: mockExtentBlkSize, + dev: bytes.NewReader(mockDisk), }, }, } fileData := writeTree(&mockExtentFile.regFile.inode, mockDisk, node0, mockExtentBlkSize) - r := bytes.NewReader(mockDisk) - if err := mockExtentFile.buildExtTree(r, mockExtentBlkSize); err != nil { + if err := mockExtentFile.buildExtTree(); err != nil { t.Fatalf("inode.buildExtTree failed: %v", err) } - return r, mockExtentFile, fileData + return mockExtentFile, fileData } // writeTree writes the tree represented by `root` to the inode and disk. It diff --git a/pkg/sentry/fs/ext/filesystem.go b/pkg/sentry/fs/ext/filesystem.go index 12aeb5dac..45b43b9e2 100644 --- a/pkg/sentry/fs/ext/filesystem.go +++ b/pkg/sentry/fs/ext/filesystem.go @@ -34,12 +34,11 @@ type filesystem struct { // mu serializes changes to the Dentry tree. mu sync.RWMutex - // dev is the io.ReaderAt for the underlying fs device. It does not require - // protection because io.ReaderAt permits concurrent read calls to it. It - // translates to the pread syscall which passes on the read request directly - // to the device driver. Device drivers are intelligent in serving multiple - // concurrent read requests in the optimal order (taking locality into - // consideration). + // dev represents the underlying fs device. It does not require protection + // because io.ReaderAt permits concurrent read calls to it. It translates to + // the pread syscall which passes on the read request directly to the device + // driver. Device drivers are intelligent in serving multiple concurrent read + // requests in the optimal order (taking locality into consideration). dev io.ReaderAt // inodeCache maps absolute inode numbers to the corresponding Inode struct. @@ -69,7 +68,7 @@ func (fs *filesystem) getOrCreateInode(ctx context.Context, inodeNum uint32) (*i return in, nil } - in, err := newInode(ctx, fs.dev, fs.sb, fs.bgs, inodeNum) + in, err := newInode(ctx, fs, inodeNum) if err != nil { return nil, err } diff --git a/pkg/sentry/fs/ext/inline_file.go b/pkg/sentry/fs/ext/inline_file.go index b9adfe548..67a538ba0 100644 --- a/pkg/sentry/fs/ext/inline_file.go +++ b/pkg/sentry/fs/ext/inline_file.go @@ -24,14 +24,8 @@ type inlineFile struct { regFile regularFile } -// Compiles only if inlineFile implements fileReader. -var _ fileReader = (*inlineFile)(nil) - -// getFileReader implements fileReader.getFileReader. -func (f *inlineFile) getFileReader(_ io.ReaderAt, _ uint64, offset uint64) io.Reader { - diskInode := f.regFile.inode.diskInode - return &inlineReader{offset: offset, data: diskInode.Data()[:diskInode.Size()]} -} +// Compiles only if inlineFile implements io.ReaderAt. +var _ io.ReaderAt = (*inlineFile)(nil) // newInlineFile is the inlineFile constructor. func newInlineFile(regFile regularFile) *inlineFile { @@ -40,27 +34,22 @@ func newInlineFile(regFile regularFile) *inlineFile { return file } -// inlineReader implements io.Reader which can read the underlying data. This -// is not thread safe. -type inlineReader struct { - offset uint64 - data []byte -} - -// Compiles only if inlineReader implements io.Reader. -var _ io.Reader = (*inlineReader)(nil) - -// Read implements io.Reader.Read. -func (r *inlineReader) Read(dst []byte) (int, error) { +// ReadAt implements io.ReaderAt.ReadAt. +func (f *inlineFile) ReadAt(dst []byte, off int64) (int, error) { if len(dst) == 0 { return 0, nil } - if int(r.offset) >= len(r.data) { + size := f.regFile.inode.diskInode.Size() + if uint64(off) >= size { return 0, io.EOF } - n := copy(dst, r.data[r.offset:]) - r.offset += uint64(n) + to := uint64(off) + uint64(len(dst)) + if to > size { + to = size + } + + n := copy(dst, f.regFile.inode.diskInode.Data()[off:to]) return n, nil } diff --git a/pkg/sentry/fs/ext/inode.go b/pkg/sentry/fs/ext/inode.go index 00e022953..364980e4c 100644 --- a/pkg/sentry/fs/ext/inode.go +++ b/pkg/sentry/fs/ext/inode.go @@ -46,6 +46,12 @@ type inode struct { // identify inodes within the ext filesystem. inodeNum uint32 + // dev represents the underlying device. Same as filesystem.dev. + dev io.ReaderAt + + // blkSize is the fs data block size. Same as filesystem.sb.BlockSize(). + blkSize uint64 + // diskInode gives us access to the inode struct on disk. Immutable. diskInode disklayout.Inode @@ -86,12 +92,12 @@ func (in *inode) decRef(fs *filesystem) { // newInode is the inode constructor. Reads the inode off disk. Identifies // inodes based on the absolute inode number on disk. -func newInode(ctx context.Context, dev io.ReaderAt, sb disklayout.SuperBlock, bgs []disklayout.BlockGroup, inodeNum uint32) (*inode, error) { +func newInode(ctx context.Context, fs *filesystem, inodeNum uint32) (*inode, error) { if inodeNum == 0 { panic("inode number 0 on ext filesystems is not possible") } - inodeRecordSize := sb.InodeSize() + inodeRecordSize := fs.sb.InodeSize() var diskInode disklayout.Inode if inodeRecordSize == disklayout.OldInodeSize { diskInode = &disklayout.InodeOld{} @@ -100,12 +106,12 @@ func newInode(ctx context.Context, dev io.ReaderAt, sb disklayout.SuperBlock, bg } // Calculate where the inode is actually placed. - inodesPerGrp := sb.InodesPerGroup() - blkSize := sb.BlockSize() - inodeTableOff := bgs[getBGNum(inodeNum, inodesPerGrp)].InodeTable() * blkSize + inodesPerGrp := fs.sb.InodesPerGroup() + blkSize := fs.sb.BlockSize() + inodeTableOff := fs.bgs[getBGNum(inodeNum, inodesPerGrp)].InodeTable() * blkSize inodeOff := inodeTableOff + uint64(uint32(inodeRecordSize)*getBGOff(inodeNum, inodesPerGrp)) - if err := readFromDisk(dev, int64(inodeOff), diskInode); err != nil { + if err := readFromDisk(fs.dev, int64(inodeOff), diskInode); err != nil { return nil, err } @@ -113,18 +119,20 @@ func newInode(ctx context.Context, dev io.ReaderAt, sb disklayout.SuperBlock, bg inode := inode{ refs: 1, inodeNum: inodeNum, + dev: fs.dev, + blkSize: blkSize, diskInode: diskInode, } switch diskInode.Mode().FileType() { case linux.ModeSymlink: - f, err := newSymlink(dev, blkSize, inode) + f, err := newSymlink(inode) if err != nil { return nil, err } return &f.inode, nil case linux.ModeRegular: - f, err := newRegularFile(dev, blkSize, inode) + f, err := newRegularFile(inode) if err != nil { return nil, err } diff --git a/pkg/sentry/fs/ext/regular_file.go b/pkg/sentry/fs/ext/regular_file.go index b48f61795..fb1bd38ef 100644 --- a/pkg/sentry/fs/ext/regular_file.go +++ b/pkg/sentry/fs/ext/regular_file.go @@ -18,20 +18,6 @@ import ( "io" ) -// fileReader is used to abstact away the complexity of how the file data is -// stored under the hood. Provides a method to get a file reader which can be -// used to read file data without worrying about how it is organized on disk. -type fileReader interface { - - // getFileReader returns a Reader implementation which can be used to read a - // file. It abstracts away the complexity of how the file is actually - // organized on disk. The reader is initialized with the passed offset. - // - // This reader is not meant to be retained across Read operations as it needs - // to be reinitialized with the correct offset for every Read. - getFileReader(dev io.ReaderAt, blkSize uint64, offset uint64) io.Reader -} - // regularFile represents a regular file's inode. This too follows the // inheritance pattern prevelant in the vfs layer described in // pkg/sentry/vfs/README.md. @@ -40,12 +26,12 @@ type regularFile struct { // This is immutable. The first field of fileReader implementations must be // regularFile to ensure temporality. - impl fileReader + impl io.ReaderAt } // newRegularFile is the regularFile constructor. It figures out what kind of // file this is and initializes the fileReader. -func newRegularFile(dev io.ReaderAt, blkSize uint64, inode inode) (*regularFile, error) { +func newRegularFile(inode inode) (*regularFile, error) { regFile := regularFile{ inode: inode, } @@ -53,7 +39,7 @@ func newRegularFile(dev io.ReaderAt, blkSize uint64, inode inode) (*regularFile, inodeFlags := inode.diskInode.Flags() if inodeFlags.Extents { - file, err := newExtentFile(dev, blkSize, regFile) + file, err := newExtentFile(regFile) if err != nil { return nil, err } @@ -72,7 +58,7 @@ func newRegularFile(dev io.ReaderAt, blkSize uint64, inode inode) (*regularFile, return &file.regFile, nil } - file, err := newBlockMapFile(blkSize, regFile) + file, err := newBlockMapFile(regFile) if err != nil { return nil, err } diff --git a/pkg/sentry/fs/ext/symlink.go b/pkg/sentry/fs/ext/symlink.go index 6a55c1a7b..9f498d989 100644 --- a/pkg/sentry/fs/ext/symlink.go +++ b/pkg/sentry/fs/ext/symlink.go @@ -15,8 +15,6 @@ package ext import ( - "io" - "gvisor.dev/gvisor/pkg/syserror" ) @@ -28,7 +26,7 @@ type symlink struct { // newSymlink is the symlink constructor. It reads out the symlink target from // the inode (however it might have been stored). -func newSymlink(dev io.ReaderAt, blkSize uint64, inode inode) (*symlink, error) { +func newSymlink(inode inode) (*symlink, error) { var file *symlink var link []byte @@ -39,14 +37,13 @@ func newSymlink(dev io.ReaderAt, blkSize uint64, inode inode) (*symlink, error) link = inode.diskInode.Data()[:size] } else { // Create a regular file out of this inode and read out the target. - regFile, err := newRegularFile(dev, blkSize, inode) + regFile, err := newRegularFile(inode) if err != nil { return nil, err } link = make([]byte, size) - reader := regFile.impl.getFileReader(dev, blkSize, 0) - if _, err := io.ReadFull(reader, link); err != nil { + if n, _ := regFile.impl.ReadAt(link, 0); uint64(n) < size { return nil, syserror.EIO } } |