diff --git a/pkg/sentry/fs/dirent.go b/pkg/sentry/fs/dirent.go index f81f7d627..dd2b4e589 100644 --- a/pkg/sentry/fs/dirent.go +++ b/pkg/sentry/fs/dirent.go @@ -773,6 +773,11 @@ func (d *Dirent) CreateHardLink(ctx context.Context, root *Dirent, target *Diren return syscall.EXDEV } + // Directories are never linkable. See fs/namei.c:vfs_link. + if IsDir(target.Inode.StableAttr) { + return syscall.EPERM + } + return d.genericCreate(ctx, root, name, func() error { if err := d.Inode.CreateHardLink(ctx, d, target, name); err != nil { return err diff --git a/pkg/sentry/syscalls/linux/sys_file.go b/pkg/sentry/syscalls/linux/sys_file.go index 3e28d4b8a..97881a1f5 100644 --- a/pkg/sentry/syscalls/linux/sys_file.go +++ b/pkg/sentry/syscalls/linux/sys_file.go @@ -1122,15 +1122,32 @@ func Symlinkat(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Sys // // This corresponds to Linux's fs/namei.c:may_linkat. func mayLinkAt(t *kernel.Task, target *fs.Inode) error { + // Linux will impose the following restrictions on hard links only if + // sysctl_protected_hardlinks is enabled. The kernel disables this + // setting by default for backward compatibility (see commit + // 561ec64ae67e), but also recommends that distributions enable it (and + // Debian does: + // https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=889098). + // + // gVisor currently behaves as though sysctl_protected_hardlinks is + // always enabled, and thus imposes the following restrictions on hard + // links. + // Technically Linux is more restrictive in 3.11.10 (requires CAP_FOWNER in // root user namespace); this is from the later f2ca379642d7 "namei: permit // linking with CAP_FOWNER in userns". - if !target.CheckOwnership(t) { - return syserror.EPERM + if target.CheckOwnership(t) { + // fs/namei.c:may_linkat: "Source inode owner (or CAP_FOWNER) + // can hardlink all they like." + return nil } - // Check that the target is not a directory and that permissions are okay. - if fs.IsDir(target.StableAttr) || target.CheckPermission(t, fs.PermMask{Read: true, Write: true}) != nil { + // If we are not the owner, then the file must be regular and have + // Read+Write permissions. + if !fs.IsRegular(target.StableAttr) { + return syserror.EPERM + } + if target.CheckPermission(t, fs.PermMask{Read: true, Write: true}) != nil { return syserror.EPERM }