netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Fix tmpfs accounting bug in Allocate().

Browse Source 
FileRangeSet.Fill() can fill in gaps partially and fail. Earlier, we were
over-correcting on failure by unaccounting everything that was reserved for the
operation. But that can lead to accounting errors in case of partial Fill()
failures. In such a case, when the file is later deleted, we would again
unaccount all the pages that were allocated via the partial Fill(), which will
causes accounting to go negative and sentry will panic.

Also did some refactoring for clearer code.

PiperOrigin-RevId: 493935801
This commit is contained in:
Ayush Ranjan
2022-12-08 10:41:25 -08:00
committed by gVisor bot
parent 5c59bdb8f1
commit f14b4bb3ca
2 changed files with 25 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/fsimpl/tmpfs/filesystem.go
+7 -6
View File
@@ -949,13 +949,14 @@ func (fs *filesystem) MountOptions() string {
return fs.mopts
}
// checkFillAllocation checks if pages allocated by Fill() and PagesToFill()
// are consistent.
func (fs *filesystem) checkFillAllocation(pagesReqd, pagesAlloced uint64) {
if pagesReqd < pagesAlloced {
panic(fmt.Sprintf("More pages were allocated by Fill() than PagesToFill() had reported: pagesReqd=%d, pagesAlloced=%d", pagesReqd, pagesAlloced))
// adjustPageAcct adjusts the accounting done against filesystem size limit in
// case there is any discrepency between the number of pages reserved vs the
// number of pages actually allocated.
func (fs *filesystem) adjustPageAcct(reserved, alloced uint64) {
if reserved < alloced {
panic(fmt.Sprintf("More pages were allocated than the pages reserved: reserved=%d, alloced=%d", reserved, alloced))
}
if pagesDiff := pagesReqd - pagesAlloced; pagesDiff > 0 {
if pagesDiff := reserved - alloced; pagesDiff > 0 {
fs.unaccountPages(pagesDiff)
}
}
pkg/sentry/fsimpl/tmpfs/regular_file.go
+18 -15
View File
@@ -300,13 +300,13 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap.
if optional.End > pgend {
optional.End = pgend
}
pagesReqd := rf.data.PagesToFill(required, optional)
if !rf.inode.fs.accountPages(pagesReqd) {
// If we can not accommodate pagesReqd pages, then retry with just
pagesToFill := rf.data.PagesToFill(required, optional)
if !rf.inode.fs.accountPages(pagesToFill) {
// If we can not accommodate pagesToFill pages, then retry with just
// the required range. Because optional may be larger than required.
// Only error out if even the required range can not be allocated for.
pagesReqd = rf.data.PagesToFill(required, required)
if !rf.inode.fs.accountPages(pagesReqd) {
pagesToFill = rf.data.PagesToFill(required, required)
if !rf.inode.fs.accountPages(pagesToFill) {
return nil, &memmap.BusError{linuxerr.ENOSPC}
}
optional = required
@@ -315,7 +315,9 @@ func (rf *regularFile) Translate(ctx context.Context, required, optional memmap.
// Newly-allocated pages are zeroed, so we don't need to do anything.
return dsts.NumBytes(), nil
})
rf.inode.fs.checkFillAllocation(pagesReqd, pagesAlloced)
// rf.data.Fill() may fail mid-way. We still want to account any pages that
// were allocated, irrespective of an error.
rf.inode.fs.adjustPageAcct(pagesToFill, pagesAlloced)
var ts []memmap.Translation
var translatedEnd uint64
@@ -383,8 +385,8 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint
return linuxerr.EFBIG
}
required := memmap.MappableRange{Start: uint64(pgstartaddr), End: uint64(pgendaddr)}
pagesReqd := f.data.PagesToFill(required, required)
if !f.inode.fs.accountPages(pagesReqd) {
pagesToFill := f.data.PagesToFill(required, required)
if !f.inode.fs.accountPages(pagesToFill) {
return linuxerr.ENOSPC
}
// Pass populate = true here despite the fact that we don't touch these pages
@@ -394,11 +396,12 @@ func (fd *regularFileFD) Allocate(ctx context.Context, mode, offset, length uint
// Newly-allocated pages are zeroed, so we don't need to do anything.
return dsts.NumBytes(), nil
})
// f.data.Fill() may fail mid-way. We still want to account any pages that
// were allocated, irrespective of an error.
f.inode.fs.adjustPageAcct(pagesToFill, pagesAlloced)
if err != nil && err != io.EOF {
f.inode.fs.unaccountPages(pagesReqd)
return err
}
f.inode.fs.checkFillAllocation(pagesReqd, pagesAlloced)
oldSize := f.size.Load()
if oldSize >= newSize {
@@ -719,9 +722,9 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64,
case gap.Ok():
// Allocate memory for the write.
gapMR := gap.Range().Intersect(pgMR)
pagesReqd := gapMR.Length() / hostarch.PageSize
pagesAlloced := rw.file.inode.fs.accountPagesPartial(pagesReqd)
if pagesAlloced == 0 {
pagesToFill := gapMR.Length() / hostarch.PageSize
pagesReserved := rw.file.inode.fs.accountPagesPartial(pagesToFill)
if pagesReserved == 0 {
if done == 0 {
retErr = linuxerr.ENOSPC
goto exitLoop
@@ -729,11 +732,11 @@ func (rw *regularFileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64,
retErr = nil
goto exitLoop
}
gapMR.End = gapMR.Start + (hostarch.PageSize * pagesAlloced)
gapMR.End = gapMR.Start + (hostarch.PageSize * pagesReserved)
fr, err := rw.file.memFile.Allocate(gapMR.Length(), pgalloc.AllocOpts{Kind: rw.file.memoryUsageKind})
if err != nil {
retErr = err
rw.file.inode.fs.unaccountPages(pagesAlloced)
rw.file.inode.fs.unaccountPages(pagesReserved)
goto exitLoop
}