From f8f12199af64189db7dd98b8bc52a48b7fb029b9 Mon Sep 17 00:00:00 2001 From: Ayush Ranjan Date: Wed, 13 Mar 2024 14:06:47 -0700 Subject: [PATCH] Error out when attempting to hard link synthetic files in non-synthetic dir. This is consistent with attempting to hard link a synthetic file in a synthetic directory (fs.LinkAt() => fs.doCreateAt(..., createInSyntheticDir=nil)), which should result in error. Due to #6739, fsimpl/gofer does not support hard links correctly since it does not have an inode abstraction. All inode fields are embedded in the dentry. So inode attributes and file data of hard linked files can go out of sync. In remote_revalidating mode, at least the inode attributes of non-synthetic files are refreshed on each access. But since synthetic files don't exist on the remote filesystem, there is no way to sync hard linked synthetic dentries. We'd have to wait for #6739 to be resolved to add such support. For now returning EOPNOTSUPP is most consistent. Fixes #10143. PiperOrigin-RevId: 615537813 --- pkg/sentry/fsimpl/gofer/dentry_impl.go | 1 + pkg/sentry/fsimpl/gofer/directfs_dentry.go | 2 ++ pkg/sentry/fsimpl/gofer/filesystem.go | 4 ++++ pkg/sentry/fsimpl/gofer/lisafs_dentry.go | 2 ++ 4 files changed, 9 insertions(+) diff --git a/pkg/sentry/fsimpl/gofer/dentry_impl.go b/pkg/sentry/fsimpl/gofer/dentry_impl.go index 0f264245c..a1a62fe1f 100644 --- a/pkg/sentry/fsimpl/gofer/dentry_impl.go +++ b/pkg/sentry/fsimpl/gofer/dentry_impl.go @@ -352,6 +352,7 @@ func (d *dentry) mknod(ctx context.Context, name string, creds *auth.Credentials // Preconditions: // - !d.isSynthetic(). +// - !target.isSynthetic(). // - d.fs.renameMu must be locked. func (d *dentry) link(ctx context.Context, target *dentry, name string) (*dentry, error) { switch dt := d.impl.(type) { diff --git a/pkg/sentry/fsimpl/gofer/directfs_dentry.go b/pkg/sentry/fsimpl/gofer/directfs_dentry.go index 165ea1cc5..6d01ccbb1 100644 --- a/pkg/sentry/fsimpl/gofer/directfs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/directfs_dentry.go @@ -538,6 +538,8 @@ func (d *directfsDentry) link(target *directfsDentry, name string) (*dentry, err } // Note that we don't need to set uid/gid for the new child. This is a hard // link. The original file already has the right owner. + // TODO(gvisor.dev/issue/6739): Hard linked dentries should share the same + // inode fields. return d.getCreatedChild(name, -1 /* uid */, -1 /* gid */, false /* isDir */) } diff --git a/pkg/sentry/fsimpl/gofer/filesystem.go b/pkg/sentry/fsimpl/gofer/filesystem.go index 8b4d0f94c..a625a9639 100644 --- a/pkg/sentry/fsimpl/gofer/filesystem.go +++ b/pkg/sentry/fsimpl/gofer/filesystem.go @@ -809,6 +809,10 @@ func (fs *filesystem) LinkAt(ctx context.Context, rp *vfs.ResolvingPath, vd vfs. if d.nlink.Load() == math.MaxUint32 { return nil, linuxerr.EMLINK } + if d.isSynthetic() { + // TODO(gvisor.dev/issue/6739): Add synthetic file hard link support. + return nil, linuxerr.EOPNOTSUPP + } return parent.link(ctx, d, name) }, nil) diff --git a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go index cc7f63a6e..245002e0e 100644 --- a/pkg/sentry/fsimpl/gofer/lisafs_dentry.go +++ b/pkg/sentry/fsimpl/gofer/lisafs_dentry.go @@ -425,6 +425,8 @@ func (d *lisafsDentry) link(ctx context.Context, target *lisafsDentry, name stri if err != nil { return nil, err } + // TODO(gvisor.dev/issue/6739): Hard linked dentries should share the same + // inode fields. return d.newChildDentry(ctx, &linkInode, name) }