2 files changed, 26 insertions, 4 deletions
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
 	}