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 incorrect tmpfs timestamp updates

Browse Source 
* Creation of files, directories (and other fs objects) in a directory
  should always update ctime.
* Same for removal.
* atime should not be updated on lookup, only readdir.

I've also renamed some misleading functions that update mtime and ctime.

PiperOrigin-RevId: 249115063
Change-Id: I30fa275fa7db96d01aa759ed64628c18bb3a7dc7
This commit is contained in:
Michael Pratt
2019-05-20 13:35:17 -07:00
committed by Shentubot
parent 4a842836e5
commit 6588427451
5 changed files with 87 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/sentry/fs/fsutil/inode.go
+10
View File
@@ -192,6 +192,16 @@ func (i *InodeSimpleAttributes) NotifyStatusChange(ctx context.Context) {
i.mu.Unlock()
}
// NotifyModificationAndStatusChange updates the modification and status change
// times.
func (i *InodeSimpleAttributes) NotifyModificationAndStatusChange(ctx context.Context) {
i.mu.Lock()
now := ktime.NowFromContext(ctx)
i.unstable.ModificationTime = now
i.unstable.StatusChangeTime = now
i.mu.Unlock()
}
// InodeSimpleExtendedAttributes implements
// fs.InodeOperations.{Get,Set,List}xattr.
//
pkg/sentry/fs/fsutil/inode_cached.go
+14 -14
View File
@@ -299,7 +299,7 @@ func (c *CachingInodeOperations) Truncate(ctx context.Context, inode *fs.Inode,
}
oldSize := c.attr.Size
c.attr.Size = size
c.touchModificationTimeLocked(now)
c.touchModificationAndStatusChangeTimeLocked(now)
// We drop c.dataMu here so that we can lock c.mapsMu and invalidate
// mappings below. This allows concurrent calls to Read/Translate/etc.
@@ -360,7 +360,7 @@ func (c *CachingInodeOperations) Allocate(ctx context.Context, offset, length in
}
c.attr.Size = newSize
c.touchModificationTimeLocked(now)
c.touchModificationAndStatusChangeTimeLocked(now)
return nil
}
@@ -394,19 +394,19 @@ func (c *CachingInodeOperations) WriteOut(ctx context.Context, inode *fs.Inode)
return c.backingFile.Sync(ctx)
}
// IncLinks increases the link count and updates cached access time.
// IncLinks increases the link count and updates cached modification time.
func (c *CachingInodeOperations) IncLinks(ctx context.Context) {
c.attrMu.Lock()
c.attr.Links++
c.touchModificationTimeLocked(ktime.NowFromContext(ctx))
c.touchModificationAndStatusChangeTimeLocked(ktime.NowFromContext(ctx))
c.attrMu.Unlock()
}
// DecLinks decreases the link count and updates cached access time.
// DecLinks decreases the link count and updates cached modification time.
func (c *CachingInodeOperations) DecLinks(ctx context.Context) {
c.attrMu.Lock()
c.attr.Links--
c.touchModificationTimeLocked(ktime.NowFromContext(ctx))
c.touchModificationAndStatusChangeTimeLocked(ktime.NowFromContext(ctx))
c.attrMu.Unlock()
}
@@ -432,19 +432,19 @@ func (c *CachingInodeOperations) touchAccessTimeLocked(now time.Time) {
c.dirtyAttr.AccessTime = true
}
// TouchModificationTime updates the cached modification and status change time
// in-place to the current time.
func (c *CachingInodeOperations) TouchModificationTime(ctx context.Context) {
// TouchModificationAndStatusChangeTime updates the cached modification and
// status change times in-place to the current time.
func (c *CachingInodeOperations) TouchModificationAndStatusChangeTime(ctx context.Context) {
c.attrMu.Lock()
c.touchModificationTimeLocked(ktime.NowFromContext(ctx))
c.touchModificationAndStatusChangeTimeLocked(ktime.NowFromContext(ctx))
c.attrMu.Unlock()
}
// touchModificationTimeLocked updates the cached modification and status
// change time in-place to the current time.
// touchModificationAndStatusChangeTimeLocked updates the cached modification
// and status change times in-place to the current time.
//
// Preconditions: c.attrMu is locked for writing.
func (c *CachingInodeOperations) touchModificationTimeLocked(now time.Time) {
func (c *CachingInodeOperations) touchModificationAndStatusChangeTimeLocked(now time.Time) {
c.attr.ModificationTime = now
c.dirtyAttr.ModificationTime = true
c.attr.StatusChangeTime = now
@@ -554,7 +554,7 @@ func (c *CachingInodeOperations) Write(ctx context.Context, src usermem.IOSequen
c.attrMu.Lock()
// Compare Linux's mm/filemap.c:__generic_file_write_iter() => file_update_time().
c.touchModificationTimeLocked(ktime.NowFromContext(ctx))
c.touchModificationAndStatusChangeTimeLocked(ktime.NowFromContext(ctx))
n, err := src.CopyInTo(ctx, &inodeReadWriter{ctx, c, offset})
c.attrMu.Unlock()
return n, err
pkg/sentry/fs/gofer/path.go
+9 -7
View File
@@ -113,7 +113,7 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string
return nil, err
}
i.touchModificationTime(ctx, dir)
i.touchModificationAndStatusChangeTime(ctx, dir)
// Get an unopened p9.File for the file we created so that it can be cloned
// and re-opened multiple times after creation, while also getting its
@@ -167,7 +167,7 @@ func (i *inodeOperations) CreateLink(ctx context.Context, dir *fs.Inode, oldname
if _, err := i.fileState.file.symlink(ctx, oldname, newname, p9.UID(owner.UID), p9.GID(owner.GID)); err != nil {
return err
}
i.touchModificationTime(ctx, dir)
i.touchModificationAndStatusChangeTime(ctx, dir)
return nil
}
@@ -189,7 +189,7 @@ func (i *inodeOperations) CreateHardLink(ctx context.Context, inode *fs.Inode, t
// Increase link count.
targetOpts.cachingInodeOps.IncLinks(ctx)
}
i.touchModificationTime(ctx, inode)
i.touchModificationAndStatusChangeTime(ctx, inode)
return nil
}
@@ -205,6 +205,8 @@ func (i *inodeOperations) CreateDirectory(ctx context.Context, dir *fs.Inode, s
}
if i.session().cachePolicy.cacheUAttrs(dir) {
// Increase link count.
//
// N.B. This will update the modification time.
i.cachingInodeOps.IncLinks(ctx)
}
if i.session().cachePolicy.cacheReaddir() {
@@ -246,7 +248,7 @@ func (i *inodeOperations) Bind(ctx context.Context, dir *fs.Inode, name string,
// We're not going to use this file.
hostFile.Close()
i.touchModificationTime(ctx, dir)
i.touchModificationAndStatusChangeTime(ctx, dir)
// Get the attributes of the file to create inode key.
qid, mask, attr, err := getattr(ctx, newFile)
@@ -317,7 +319,7 @@ func (i *inodeOperations) Remove(ctx context.Context, dir *fs.Inode, name string
if removeSocket {
i.session().endpoints.remove(key)
}
i.touchModificationTime(ctx, dir)
i.touchModificationAndStatusChangeTime(ctx, dir)
return nil
}
@@ -397,9 +399,9 @@ func (i *inodeOperations) Rename(ctx context.Context, inode *fs.Inode, oldParent
return nil
}
func (i *inodeOperations) touchModificationTime(ctx context.Context, inode *fs.Inode) {
func (i *inodeOperations) touchModificationAndStatusChangeTime(ctx context.Context, inode *fs.Inode) {
if i.session().cachePolicy.cacheUAttrs(inode) {
i.cachingInodeOps.TouchModificationTime(ctx)
i.cachingInodeOps.TouchModificationAndStatusChangeTime(ctx)
}
if i.session().cachePolicy.cacheReaddir() {
// Invalidate readdir cache.
pkg/sentry/fs/ramfs/dir.go
+20 -21
View File
@@ -112,7 +112,7 @@ func NewDir(ctx context.Context, contents map[string]*fs.Inode, owner fs.FileOwn
}
// addChildLocked add the child inode, inheriting its reference.
func (d *Dir) addChildLocked(name string, inode *fs.Inode) {
func (d *Dir) addChildLocked(ctx context.Context, name string, inode *fs.Inode) {
d.children[name] = inode
d.dentryMap.Add(name, fs.DentAttr{
Type: inode.StableAttr.Type,
@@ -123,18 +123,25 @@ func (d *Dir) addChildLocked(name string, inode *fs.Inode) {
// corresponding to '..' from the subdirectory.
if fs.IsDir(inode.StableAttr) {
d.AddLink()
// ctime updated below.
}
// Given we're now adding this inode to the directory we must also
// increase its link count. Similarly we decremented it in removeChildLocked.
// increase its link count. Similarly we decrement it in removeChildLocked.
//
// Changing link count updates ctime.
inode.AddLink()
inode.InodeOperations.NotifyStatusChange(ctx)
// We've change the directory. This always updates our mtime and ctime.
d.NotifyModificationAndStatusChange(ctx)
}
// AddChild adds a child to this dir.
func (d *Dir) AddChild(ctx context.Context, name string, inode *fs.Inode) {
d.mu.Lock()
defer d.mu.Unlock()
d.addChildLocked(name, inode)
d.addChildLocked(ctx, name, inode)
}
// FindChild returns (child, true) if the directory contains name.
@@ -179,14 +186,18 @@ func (d *Dir) removeChildLocked(ctx context.Context, name string) (*fs.Inode, er
// link count which was the child's ".." directory entry.
if fs.IsDir(inode.StableAttr) {
d.DropLink()
// ctime changed below.
}
// Update ctime.
inode.InodeOperations.NotifyStatusChange(ctx)
// Given we're now removing this inode to the directory we must also
// decrease its link count. Similarly it is increased in addChildLocked.
//
// Changing link count updates ctime.
inode.DropLink()
inode.InodeOperations.NotifyStatusChange(ctx)
// We've change the directory. This always updates our mtime and ctime.
d.NotifyModificationAndStatusChange(ctx)
return inode, nil
}
@@ -263,8 +274,6 @@ func (d *Dir) Lookup(ctx context.Context, _ *fs.Inode, p string) (*fs.Dirent, er
// walkLocked must be called with d.mu held.
func (d *Dir) walkLocked(ctx context.Context, p string) (*fs.Inode, error) {
d.NotifyAccess(ctx)
// Lookup a child node.
if inode, ok := d.children[p]; ok {
return inode, nil
@@ -290,8 +299,7 @@ func (d *Dir) createInodeOperationsCommon(ctx context.Context, name string, make
return nil, err
}
d.addChildLocked(name, inode)
d.NotifyModification(ctx)
d.addChildLocked(ctx, name, inode)
return inode, nil
}
@@ -342,11 +350,7 @@ func (d *Dir) CreateHardLink(ctx context.Context, dir *fs.Inode, target *fs.Inod
target.IncRef()
// The link count will be incremented in addChildLocked.
d.addChildLocked(name, target)
d.NotifyModification(ctx)
// Update ctime.
target.InodeOperations.NotifyStatusChange(ctx)
d.addChildLocked(ctx, name, target)
return nil
}
@@ -359,8 +363,6 @@ func (d *Dir) CreateDirectory(ctx context.Context, dir *fs.Inode, name string, p
_, err := d.createInodeOperationsCommon(ctx, name, func() (*fs.Inode, error) {
return d.NewDir(ctx, dir, perms)
})
// TODO(nlacasse): Support updating status times, as those should be
// updated by links.
return err
}
@@ -526,10 +528,7 @@ func Rename(ctx context.Context, oldParent fs.InodeOperations, oldName string, n
// Do the swap.
n := op.children[oldName]
op.removeChildLocked(ctx, oldName)
np.addChildLocked(newName, n)
// Update ctime.
n.InodeOperations.NotifyStatusChange(ctx)
np.addChildLocked(ctx, newName, n)
return nil
}
test/syscalls/linux/stat_times.cc
+34
View File
@@ -263,6 +263,40 @@ TEST(StatTimesTest, DirList) {
CtimeEffect::Unchanged);
}
// Creating a file in a directory changes mtime and ctime.
TEST(StatTimesTest, DirCreateFile) {
const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TempPath file;
auto fn = [&] {
file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path()));
};
CheckTimes(dir, fn, AtimeEffect::Unchanged, MtimeEffect::Changed,
CtimeEffect::Changed);
}
// Creating a directory in a directory changes mtime and ctime.
TEST(StatTimesTest, DirCreateDir) {
const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TempPath dir2;
auto fn = [&] {
dir2 = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(dir.path()));
};
CheckTimes(dir, fn, AtimeEffect::Unchanged, MtimeEffect::Changed,
CtimeEffect::Changed);
}
// Removing a file from a directory changes mtime and ctime.
TEST(StatTimesTest, DirRemoveFile) {
const TempPath dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TempPath file = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(dir.path()));
auto fn = [&] { file.reset(); };
CheckTimes(dir, fn, AtimeEffect::Unchanged, MtimeEffect::Changed,
CtimeEffect::Changed);
}
} // namespace
} // namespace testing