diff --git a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go index dc993fb3b..4cb27844d 100644 --- a/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go +++ b/pkg/sentry/fsimpl/cgroupfs/cgroupfs.go @@ -493,6 +493,7 @@ type dir struct { kernfs.InodeAttrs kernfs.InodeNotSymlink kernfs.InodeDirectoryNoNewChildren + kernfs.InodeWatches kernfs.OrderedChildren implStatFS diff --git a/pkg/sentry/fsimpl/devpts/devpts.go b/pkg/sentry/fsimpl/devpts/devpts.go index e711debcb..a17c9f677 100644 --- a/pkg/sentry/fsimpl/devpts/devpts.go +++ b/pkg/sentry/fsimpl/devpts/devpts.go @@ -152,6 +152,7 @@ type rootInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary // This holds no meaning as this inode can't be Looked up and is always valid. + kernfs.InodeWatches kernfs.OrderedChildren rootInodeRefs diff --git a/pkg/sentry/fsimpl/devpts/master.go b/pkg/sentry/fsimpl/devpts/master.go index c2faa0b7b..738f5c9c0 100644 --- a/pkg/sentry/fsimpl/devpts/master.go +++ b/pkg/sentry/fsimpl/devpts/master.go @@ -38,6 +38,7 @@ type masterInode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches locks vfs.FileLocks diff --git a/pkg/sentry/fsimpl/devpts/replica.go b/pkg/sentry/fsimpl/devpts/replica.go index d1ddf0045..24d925a8f 100644 --- a/pkg/sentry/fsimpl/devpts/replica.go +++ b/pkg/sentry/fsimpl/devpts/replica.go @@ -37,6 +37,7 @@ type replicaInode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches locks vfs.FileLocks diff --git a/pkg/sentry/fsimpl/fuse/fusefs.go b/pkg/sentry/fsimpl/fuse/fusefs.go index 8ce53b704..9dc2ef6bd 100644 --- a/pkg/sentry/fsimpl/fuse/fusefs.go +++ b/pkg/sentry/fsimpl/fuse/fusefs.go @@ -305,6 +305,7 @@ type inode struct { kernfs.InodeAttrs kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink + kernfs.InodeWatches kernfs.OrderedChildren // the owning filesystem. fs is immutable. diff --git a/pkg/sentry/fsimpl/host/host.go b/pkg/sentry/fsimpl/host/host.go index f33433c91..38d8993fa 100644 --- a/pkg/sentry/fsimpl/host/host.go +++ b/pkg/sentry/fsimpl/host/host.go @@ -99,6 +99,7 @@ type inode struct { kernfs.InodeNotSymlink kernfs.CachedMappable kernfs.InodeTemporary // This holds no meaning as this inode can't be Looked up and is always valid. + kernfs.InodeWatches locks vfs.FileLocks diff --git a/pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go b/pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go index 17602c63c..a60d589db 100644 --- a/pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go +++ b/pkg/sentry/fsimpl/kernfs/dynamic_bytes_file.go @@ -40,6 +40,7 @@ type DynamicBytesFile struct { InodeNoopRefCount InodeNotDirectory InodeNotSymlink + InodeWatches locks vfs.FileLocks // data can additionally implement vfs.WritableDynamicBytesSource to support diff --git a/pkg/sentry/fsimpl/kernfs/filesystem.go b/pkg/sentry/fsimpl/kernfs/filesystem.go index 47a071ceb..f42687218 100644 --- a/pkg/sentry/fsimpl/kernfs/filesystem.go +++ b/pkg/sentry/fsimpl/kernfs/filesystem.go @@ -395,6 +395,8 @@ func (fs *Filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. if err != nil { return err } + parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE, 0, vfs.InodeEvent, false /* unlinked */) + d.inode.Watches().Notify(ctx, "", linux.IN_ATTRIB, 0, vfs.InodeEvent, false /* unlinked */) var child Dentry child.Init(fs, childI) parent.insertChildLocked(pc, &child) @@ -433,6 +435,7 @@ func (fs *Filesystem) MkdirAt(ctx context.Context, rp *vfs.ResolvingPath, opts v } var child Dentry child.Init(fs, childI) + parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE|linux.IN_ISDIR, 0, vfs.InodeEvent, false /* unlinked */) parent.insertChildLocked(pc, &child) return nil } @@ -467,6 +470,7 @@ func (fs *Filesystem) MknodAt(ctx context.Context, rp *vfs.ResolvingPath, opts v if err != nil { return err } + parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE, 0, vfs.InodeEvent, false /* unlinked */) var newD Dentry newD.Init(fs, newI) parent.insertChildLocked(pc, &newD) @@ -559,6 +563,9 @@ afterTrailingSymlink: if len(pc) > linux.NAME_MAX { return nil, linuxerr.ENAMETOOLONG } + if parent.VFSDentry().IsDead() { + return nil, linuxerr.ENOENT + } // Determine whether or not we need to create a file. child, err := fs.stepExistingLocked(ctx, rp, parent, false /* mayFollowSymlinks */) if linuxerr.Equals(linuxerr.ENOENT, err) { @@ -582,6 +589,7 @@ afterTrailingSymlink: // its destruction while fs.mu is unlocked. child.IncRef() unlock() + parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE, 0, vfs.PathEvent, false /* unlinked */) fd, err := child.inode.Open(ctx, rp, &child, opts) child.DecRef(ctx) return fd, err @@ -765,7 +773,9 @@ func (fs *Filesystem) RenameAt(ctx context.Context, rp *vfs.ResolvingPath, oldPa // deferDecRef so that fs.mu and dstDir.mu are unlocked by then. fs.deferDecRef(replaced) replaceVFSD = replaced.VFSDentry() + replaced.setDeleted() } + vfs.InotifyRename(ctx, src.inode.Watches(), srcDir.inode.Watches(), dstDir.inode.Watches(), oldName, newName, src.isDir()) virtfs.CommitRenameReplaceDentry(ctx, srcVFSD, replaceVFSD) // +checklocksforce: to may be nil, that's okay. return nil } @@ -810,9 +820,11 @@ func (fs *Filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error return err } delete(parentDentry.children, d.name) + parentDentry.inode.Watches().Notify(ctx, d.name, linux.IN_DELETE|linux.IN_ISDIR, 0, vfs.InodeEvent, true /* unlinked */) // Defer decref so that fs.mu and parentDentry.dirMu are unlocked by then. fs.deferDecRef(d) virtfs.CommitDeleteDentry(ctx, vfsd) + d.setDeleted() return nil } @@ -820,15 +832,24 @@ func (fs *Filesystem) RmdirAt(ctx context.Context, rp *vfs.ResolvingPath) error func (fs *Filesystem) SetStatAt(ctx context.Context, rp *vfs.ResolvingPath, opts vfs.SetStatOptions) error { fs.mu.RLock() defer fs.processDeferredDecRefs(ctx) - defer fs.mu.RUnlock() d, err := fs.walkExistingLocked(ctx, rp) if err != nil { + fs.mu.RUnlock() return err } if opts.Stat.Mask == 0 { + fs.mu.RUnlock() return nil } - return d.inode.SetStat(ctx, fs.VFSFilesystem(), rp.Credentials(), opts) + err = d.inode.SetStat(ctx, fs.VFSFilesystem(), rp.Credentials(), opts) + fs.mu.RUnlock() + if err != nil { + return err + } + if ev := vfs.InotifyEventFromStatMask(opts.Stat.Mask); ev != 0 { + d.InotifyWithParent(ctx, ev, 0, vfs.InodeEvent) + } + return nil } // StatAt implements vfs.FilesystemImpl.StatAt. @@ -885,6 +906,7 @@ func (fs *Filesystem) SymlinkAt(ctx context.Context, rp *vfs.ResolvingPath, targ if err != nil { return err } + parent.inode.Watches().Notify(ctx, pc, linux.IN_CREATE, 0, vfs.InodeEvent, false /* unlinked */) var child Dentry child.Init(fs, childI) parent.insertChildLocked(pc, &child) @@ -926,9 +948,11 @@ func (fs *Filesystem) UnlinkAt(ctx context.Context, rp *vfs.ResolvingPath) error return err } delete(parentDentry.children, d.name) + vfs.InotifyRemoveChild(ctx, d.inode.Watches(), parentDentry.inode.Watches(), d.name) // Defer decref so that fs.mu and parentDentry.dirMu are unlocked by then. fs.deferDecRef(d) virtfs.CommitDeleteDentry(ctx, vfsd) + d.setDeleted() return nil } diff --git a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go index bcd019480..befa3a375 100644 --- a/pkg/sentry/fsimpl/kernfs/inode_impl_util.go +++ b/pkg/sentry/fsimpl/kernfs/inode_impl_util.go @@ -716,6 +716,7 @@ type StaticDirectory struct { InodeNoStatFS InodeNotSymlink InodeTemporary + InodeWatches OrderedChildren StaticDirectoryRefs @@ -796,3 +797,15 @@ type InodeNoStatFS struct{} func (*InodeNoStatFS) StatFS(context.Context, *vfs.Filesystem) (linux.Statfs, error) { return linux.Statfs{}, linuxerr.ENOSYS } + +// InodeWatches partially implements Inode. +// +// +stateify savable +type InodeWatches struct { + watches vfs.Watches +} + +// Watches implements Inode.Watches. +func (i *InodeWatches) Watches() *vfs.Watches { + return &i.watches +} diff --git a/pkg/sentry/fsimpl/kernfs/kernfs.go b/pkg/sentry/fsimpl/kernfs/kernfs.go index 43af31123..37c0bf108 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs.go @@ -244,6 +244,10 @@ type Dentry struct { children map[string]*Dentry inode Inode + + // If deleted is non-zero, the file represented by this dentry has been + // deleted. deleted is accessed using atomic memory operations. + deleted atomicbitops.Uint32 } // IncRef implements vfs.DentryImpl.IncRef. @@ -339,6 +343,9 @@ func (d *Dentry) cacheLocked(ctx context.Context) { d.fs.cachedDentriesLen-- d.cached = false } + if d.isDeleted() { + d.inode.Watches().HandleDeletion(ctx) + } d.destroyLocked(ctx) return } @@ -420,7 +427,6 @@ func (d *Dentry) destroyLocked(ctx context.Context) { } d.inode.DecRef(ctx) // IncRef from Init. - d.inode = nil if d.parent != nil { d.parent.decRefLocked(ctx) @@ -485,6 +491,14 @@ func (d *Dentry) VFSDentry() *vfs.Dentry { return &d.vfsd } +func (d *Dentry) isDeleted() bool { + return d.deleted.Load() != 0 +} + +func (d *Dentry) setDeleted() { + d.deleted.Store(1) +} + // isDir checks whether the dentry points to a directory inode. func (d *Dentry) isDir() bool { return d.flags.Load()&dflagsIsDir != 0 @@ -496,15 +510,23 @@ func (d *Dentry) isSymlink() bool { } // InotifyWithParent implements vfs.DentryImpl.InotifyWithParent. -// -// Although Linux technically supports inotify on pseudo filesystems (inotify -// is implemented at the vfs layer), it is not particularly useful. It is left -// unimplemented until someone actually needs it. -func (d *Dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, et vfs.EventType) {} +func (d *Dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, et vfs.EventType) { + if d.isDir() { + events |= linux.IN_ISDIR + } + + d.fs.mu.RLock() + defer d.fs.mu.RUnlock() + // The ordering below is important, Linux always notifies the parent first. + if d.parent != nil { + d.parent.inode.Watches().Notify(ctx, d.name, events, cookie, et, d.isDeleted()) + } + d.inode.Watches().Notify(ctx, "", events, cookie, et, d.isDeleted()) +} // Watches implements vfs.DentryImpl.Watches. func (d *Dentry) Watches() *vfs.Watches { - return nil + return d.inode.Watches() } // OnZeroWatches implements vfs.Dentry.OnZeroWatches. @@ -680,6 +702,9 @@ type Inode interface { // Valid should return true if this inode is still valid, or needs to // be resolved again by a call to Lookup. Valid(ctx context.Context) bool + + // Watches returns the set of inotify watches associated with this inode. + Watches() *vfs.Watches } type inodeRefs interface { diff --git a/pkg/sentry/fsimpl/kernfs/kernfs_test.go b/pkg/sentry/fsimpl/kernfs/kernfs_test.go index a2aba9321..bcc6ec94a 100644 --- a/pkg/sentry/fsimpl/kernfs/kernfs_test.go +++ b/pkg/sentry/fsimpl/kernfs/kernfs_test.go @@ -106,6 +106,7 @@ type readonlyDir struct { kernfs.InodeNoStatFS kernfs.InodeNotSymlink kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren locks vfs.FileLocks @@ -141,6 +142,7 @@ type dir struct { kernfs.InodeNotSymlink kernfs.InodeNoStatFS kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren locks vfs.FileLocks diff --git a/pkg/sentry/fsimpl/kernfs/symlink.go b/pkg/sentry/fsimpl/kernfs/symlink.go index 4adf76ce6..a127804bf 100644 --- a/pkg/sentry/fsimpl/kernfs/symlink.go +++ b/pkg/sentry/fsimpl/kernfs/symlink.go @@ -31,6 +31,7 @@ type StaticSymlink struct { InodeNoopRefCount InodeSymlink InodeNoStatFS + InodeWatches target string } diff --git a/pkg/sentry/fsimpl/kernfs/synthetic_directory.go b/pkg/sentry/fsimpl/kernfs/synthetic_directory.go index c91d23b56..ed587bf47 100644 --- a/pkg/sentry/fsimpl/kernfs/synthetic_directory.go +++ b/pkg/sentry/fsimpl/kernfs/synthetic_directory.go @@ -33,6 +33,7 @@ type syntheticDirectory struct { InodeAttrs InodeNoStatFS InodeNotSymlink + InodeWatches OrderedChildren syntheticDirectoryRefs diff --git a/pkg/sentry/fsimpl/mqfs/root.go b/pkg/sentry/fsimpl/mqfs/root.go index 922e669e2..4a3e950ba 100644 --- a/pkg/sentry/fsimpl/mqfs/root.go +++ b/pkg/sentry/fsimpl/mqfs/root.go @@ -33,6 +33,7 @@ type rootInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren locks vfs.FileLocks diff --git a/pkg/sentry/fsimpl/pipefs/pipefs.go b/pkg/sentry/fsimpl/pipefs/pipefs.go index af09195a7..4ace8e83f 100644 --- a/pkg/sentry/fsimpl/pipefs/pipefs.go +++ b/pkg/sentry/fsimpl/pipefs/pipefs.go @@ -92,6 +92,7 @@ type inode struct { kernfs.InodeNotDirectory kernfs.InodeNotSymlink kernfs.InodeNoopRefCount + kernfs.InodeWatches locks vfs.FileLocks pipe *pipe.VFSPipe diff --git a/pkg/sentry/fsimpl/proc/subtasks.go b/pkg/sentry/fsimpl/proc/subtasks.go index e04ae6660..5dec43bac 100644 --- a/pkg/sentry/fsimpl/proc/subtasks.go +++ b/pkg/sentry/fsimpl/proc/subtasks.go @@ -37,6 +37,7 @@ type subtasksInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren subtasksInodeRefs diff --git a/pkg/sentry/fsimpl/proc/task.go b/pkg/sentry/fsimpl/proc/task.go index a496b7221..de9ae935b 100644 --- a/pkg/sentry/fsimpl/proc/task.go +++ b/pkg/sentry/fsimpl/proc/task.go @@ -37,6 +37,7 @@ type taskInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren taskInodeRefs diff --git a/pkg/sentry/fsimpl/proc/task_fds.go b/pkg/sentry/fsimpl/proc/task_fds.go index 5c6412fc0..0c636aa5e 100644 --- a/pkg/sentry/fsimpl/proc/task_fds.go +++ b/pkg/sentry/fsimpl/proc/task_fds.go @@ -114,6 +114,7 @@ type fdDirInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren } @@ -197,6 +198,7 @@ type fdSymlink struct { kernfs.InodeAttrs kernfs.InodeNoopRefCount kernfs.InodeSymlink + kernfs.InodeWatches fs *filesystem task *kernel.Task @@ -256,6 +258,7 @@ type fdInfoDirInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren } diff --git a/pkg/sentry/fsimpl/proc/task_files.go b/pkg/sentry/fsimpl/proc/task_files.go index ca5f4eeb9..071597b4c 100644 --- a/pkg/sentry/fsimpl/proc/task_files.go +++ b/pkg/sentry/fsimpl/proc/task_files.go @@ -394,6 +394,7 @@ type memInode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches task *kernel.Task locks vfs.FileLocks @@ -675,6 +676,7 @@ type statusInode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches task *kernel.Task pidns *kernel.PIDNamespace @@ -906,6 +908,7 @@ type exeSymlink struct { kernfs.InodeAttrs kernfs.InodeNoopRefCount kernfs.InodeSymlink + kernfs.InodeWatches fs *filesystem task *kernel.Task @@ -978,6 +981,7 @@ type cwdSymlink struct { kernfs.InodeAttrs kernfs.InodeNoopRefCount kernfs.InodeSymlink + kernfs.InodeWatches fs *filesystem task *kernel.Task @@ -1152,6 +1156,7 @@ type namespaceInode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches locks vfs.FileLocks } diff --git a/pkg/sentry/fsimpl/proc/tasks.go b/pkg/sentry/fsimpl/proc/tasks.go index c931ad1f7..45837f4d1 100644 --- a/pkg/sentry/fsimpl/proc/tasks.go +++ b/pkg/sentry/fsimpl/proc/tasks.go @@ -43,6 +43,7 @@ type tasksInode struct { kernfs.InodeDirectoryNoNewChildren kernfs.InodeNotSymlink kernfs.InodeTemporary // This holds no meaning as this inode can't be Looked up and is always valid. + kernfs.InodeWatches kernfs.OrderedChildren tasksInodeRefs diff --git a/pkg/sentry/fsimpl/proc/tasks_files.go b/pkg/sentry/fsimpl/proc/tasks_files.go index 5168d49a6..93868fec7 100644 --- a/pkg/sentry/fsimpl/proc/tasks_files.go +++ b/pkg/sentry/fsimpl/proc/tasks_files.go @@ -37,6 +37,7 @@ type selfSymlink struct { kernfs.InodeAttrs kernfs.InodeNoopRefCount kernfs.InodeSymlink + kernfs.InodeWatches pidns *kernel.PIDNamespace } @@ -78,6 +79,7 @@ type threadSelfSymlink struct { kernfs.InodeAttrs kernfs.InodeNoopRefCount kernfs.InodeSymlink + kernfs.InodeWatches pidns *kernel.PIDNamespace } diff --git a/pkg/sentry/fsimpl/sockfs/sockfs.go b/pkg/sentry/fsimpl/sockfs/sockfs.go index dcb899936..290d55b8d 100644 --- a/pkg/sentry/fsimpl/sockfs/sockfs.go +++ b/pkg/sentry/fsimpl/sockfs/sockfs.go @@ -98,6 +98,7 @@ type inode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches } // Open implements kernfs.Inode.Open. diff --git a/pkg/sentry/fsimpl/sys/kcov.go b/pkg/sentry/fsimpl/sys/kcov.go index 51f0bf3d8..cde68ac79 100644 --- a/pkg/sentry/fsimpl/sys/kcov.go +++ b/pkg/sentry/fsimpl/sys/kcov.go @@ -41,6 +41,7 @@ type kcovInode struct { kernfs.InodeNoopRefCount kernfs.InodeNotDirectory kernfs.InodeNotSymlink + kernfs.InodeWatches implStatFS } diff --git a/pkg/sentry/fsimpl/sys/sys.go b/pkg/sentry/fsimpl/sys/sys.go index 3c6b41137..9f9be7905 100644 --- a/pkg/sentry/fsimpl/sys/sys.go +++ b/pkg/sentry/fsimpl/sys/sys.go @@ -200,6 +200,7 @@ type dir struct { kernfs.InodeNotSymlink kernfs.InodeDirectoryNoNewChildren kernfs.InodeTemporary + kernfs.InodeWatches kernfs.OrderedChildren locks vfs.FileLocks diff --git a/pkg/sentry/syscalls/linux/vfs2/inotify.go b/pkg/sentry/syscalls/linux/vfs2/inotify.go index 12f5aa91f..739be9463 100644 --- a/pkg/sentry/syscalls/linux/vfs2/inotify.go +++ b/pkg/sentry/syscalls/linux/vfs2/inotify.go @@ -116,11 +116,7 @@ func InotifyAddWatch(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kern } defer d.DecRef(t) - fd, err = ino.AddWatch(d.Dentry(), mask) - if err != nil { - return 0, nil, err - } - return uintptr(fd), nil, nil + return uintptr(ino.AddWatch(d.Dentry(), mask)), nil, nil } // InotifyRmWatch implements the inotify_rm_watch() syscall. diff --git a/pkg/sentry/vfs/anonfs.go b/pkg/sentry/vfs/anonfs.go index f34770c77..e8e7e2e0b 100644 --- a/pkg/sentry/vfs/anonfs.go +++ b/pkg/sentry/vfs/anonfs.go @@ -87,6 +87,11 @@ type anonDentry struct { vfsd Dentry name string + + // Inotify watches for this dentry. Note that anonfs doesn't allow hardlinks + // and the dentry lifetime matches exactly with the file lifetime so it is + // okay to have the watches in the dentry itself. + watches Watches } // Release implements FilesystemImpl.Release. @@ -312,15 +317,14 @@ func (d *anonDentry) DecRef(ctx context.Context) { } // InotifyWithParent implements DentryImpl.InotifyWithParent. -// -// Although Linux technically supports inotify on pseudo filesystems (inotify -// is implemented at the vfs layer), it is not particularly useful. It is left -// unimplemented until someone actually needs it. -func (d *anonDentry) InotifyWithParent(ctx context.Context, events, cookie uint32, et EventType) {} +func (d *anonDentry) InotifyWithParent(ctx context.Context, events, cookie uint32, et EventType) { + // d.parent doesn't exist. + d.watches.Notify(ctx, "", events, cookie, et, false /* unlinked */) +} // Watches implements DentryImpl.Watches. func (d *anonDentry) Watches() *Watches { - return nil + return &d.watches } // OnZeroWatches implements Dentry.OnZeroWatches. diff --git a/pkg/sentry/vfs/dentry.go b/pkg/sentry/vfs/dentry.go index 4edb5f7c3..508df80b8 100644 --- a/pkg/sentry/vfs/dentry.go +++ b/pkg/sentry/vfs/dentry.go @@ -126,12 +126,6 @@ type DentryImpl interface { // the Dentry. Dentries that are hard links to the same underlying file // share the same watches. // - // Watches may return nil if the dentry belongs to a FilesystemImpl that - // does not support inotify. If an implementation returns a non-nil watch - // set, it must always return a non-nil watch set. Likewise, if an - // implementation returns a nil watch set, it must always return a nil watch - // set. - // // The caller does not need to hold a reference on the dentry. Watches() *Watches @@ -194,9 +188,6 @@ func (d *Dentry) InotifyWithParent(ctx context.Context, events, cookie uint32, e } // Watches returns the set of inotify watches associated with d. -// -// Watches will return nil if d belongs to a FilesystemImpl that does not -// support inotify. func (d *Dentry) Watches() *Watches { return d.impl.Watches() } diff --git a/pkg/sentry/vfs/inotify.go b/pkg/sentry/vfs/inotify.go index 2a9dda9d5..aea54a5d5 100644 --- a/pkg/sentry/vfs/inotify.go +++ b/pkg/sentry/vfs/inotify.go @@ -326,7 +326,7 @@ func (i *Inotify) nextWatchIDLocked() int32 { // returns the watch descriptor returned by inotify_add_watch(2). // // The caller must hold a reference on target. -func (i *Inotify) AddWatch(target *Dentry, mask uint32) (int32, error) { +func (i *Inotify) AddWatch(target *Dentry, mask uint32) int32 { // Note: Locking this inotify instance protects the result returned by // Lookup() below. With the lock held, we know for sure the lookup result // won't become stale because it's impossible for *this* instance to @@ -335,11 +335,6 @@ func (i *Inotify) AddWatch(target *Dentry, mask uint32) (int32, error) { defer i.mu.Unlock() ws := target.Watches() - if ws == nil { - // While Linux supports inotify watches on all filesystem types, watches on - // filesystems like kernfs are not generally useful, so we do not. - return 0, linuxerr.EPERM - } // Does the target already have a watch from this inotify instance? if existing := ws.Lookup(i.id); existing != nil { newmask := mask @@ -349,12 +344,12 @@ func (i *Inotify) AddWatch(target *Dentry, mask uint32) (int32, error) { newmask |= existing.mask.Load() } existing.mask.Store(newmask) - return existing.wd, nil + return existing.wd } // No existing watch, create a new watch. w := i.newWatchLocked(target, ws, mask) - return w.wd, nil + return w.wd } // RmWatch looks up an inotify watch for the given 'wd' and configures the diff --git a/test/syscalls/linux/inotify.cc b/test/syscalls/linux/inotify.cc index 66db40c66..154a86522 100644 --- a/test/syscalls/linux/inotify.cc +++ b/test/syscalls/linux/inotify.cc @@ -2501,6 +2501,25 @@ TEST(InotifyTest, NotifyNoDeadlock) { } } +// NOTE(b/239215242): Regression test. +TEST(Inotify, KernfsBasic) { + const FileDescriptor fd = + ASSERT_NO_ERRNO_AND_VALUE(InotifyInit1(IN_NONBLOCK)); + const std::string procFile = "/proc/filesystems"; + + const int wd = ASSERT_NO_ERRNO_AND_VALUE( + InotifyAddWatch(fd.get(), procFile, IN_ALL_EVENTS)); + const FileDescriptor file1_fd = + ASSERT_NO_ERRNO_AND_VALUE(Open(procFile, O_RDONLY)); + + char buf; + EXPECT_THAT(read(file1_fd.get(), &buf, 1), SyscallSucceeds()); + + const std::vector events = + ASSERT_NO_ERRNO_AND_VALUE(DrainEvents(fd.get())); + ASSERT_THAT(events, Are({Event(IN_OPEN, wd), Event(IN_ACCESS, wd)})); +} + } // namespace } // namespace testing } // namespace gvisor