All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: James Morris <jmorris@namei.org>, "Serge E . Hallyn" <serge@hallyn.com>
Cc: "Mickaël Salaün" <mic@digikod.net>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	"Jann Horn" <jannh@google.com>,
	"John Johansen" <john.johansen@canonical.com>,
	"Kees Cook" <keescook@chromium.org>,
	"Konstantin Meskhidze" <konstantin.meskhidze@huawei.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Tetsuo Handa" <penguin-kernel@I-love.SAKURA.ne.jp>,
	linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	"Kentaro Takeda" <takedakn@nttdata.co.jp>
Subject: [PATCH v3 06/12] LSM: Remove double path_rename hook calls for RENAME_EXCHANGE
Date: Fri,  6 May 2022 18:10:56 +0200	[thread overview]
Message-ID: <20220506161102.525323-7-mic@digikod.net> (raw)
In-Reply-To: <20220506161102.525323-1-mic@digikod.net>

In order to be able to identify a file exchange with renameat2(2) and
RENAME_EXCHANGE, which will be useful for Landlock [1], propagate the
rename flags to LSMs.  This may also improve performance because of the
switch from two set of LSM hook calls to only one, and because LSMs
using this hook may optimize the double check (e.g. only one lock,
reduce the number of path walks).

AppArmor, Landlock and Tomoyo are updated to leverage this change.  This
should not change the current behavior (same check order), except
(different level of) speed boosts.

[1] https://lore.kernel.org/r/20220221212522.320243-1-mic@digikod.net

Cc: James Morris <jmorris@namei.org>
Cc: Kentaro Takeda <takedakn@nttdata.co.jp>
Cc: Serge E. Hallyn <serge@hallyn.com>
Acked-by: John Johansen <john.johansen@canonical.com>
Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reviewed-by: Paul Moore <paul@paul-moore.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
Link: https://lore.kernel.org/r/20220506161102.525323-7-mic@digikod.net
---

Changes since v2:
* Add Reviewed-by: Paul Moore.
* Format security/landlock/fs.c with clang-format.

Changes since v1:
* Import patch from
  https://lore.kernel.org/r/20220222175332.384545-1-mic@digikod.net
* Add Acked-by: Tetsuo Handa.
* Add Acked-by: John Johansen.
---
 include/linux/lsm_hook_defs.h |  2 +-
 include/linux/lsm_hooks.h     |  1 +
 security/apparmor/lsm.c       | 30 +++++++++++++++++++++++++-----
 security/landlock/fs.c        | 11 ++++++++++-
 security/security.c           |  9 +--------
 security/tomoyo/tomoyo.c      | 11 ++++++++++-
 6 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index db924fe379c9..eafa1d2489fd 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -100,7 +100,7 @@ LSM_HOOK(int, 0, path_link, struct dentry *old_dentry,
 	 const struct path *new_dir, struct dentry *new_dentry)
 LSM_HOOK(int, 0, path_rename, const struct path *old_dir,
 	 struct dentry *old_dentry, const struct path *new_dir,
-	 struct dentry *new_dentry)
+	 struct dentry *new_dentry, unsigned int flags)
 LSM_HOOK(int, 0, path_chmod, const struct path *path, umode_t mode)
 LSM_HOOK(int, 0, path_chown, const struct path *path, kuid_t uid, kgid_t gid)
 LSM_HOOK(int, 0, path_chroot, const struct path *path)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 419b5febc3ca..9acf5e368d73 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -358,6 +358,7 @@
  *	@old_dentry contains the dentry structure of the old link.
  *	@new_dir contains the path structure for parent of the new link.
  *	@new_dentry contains the dentry structure of the new link.
+ *	@flags may contain rename options such as RENAME_EXCHANGE.
  *	Return 0 if permission is granted.
  * @path_chmod:
  *	Check for permission to change a mode of the file @path. The new
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 4f0eecb67dde..900bc540656a 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -354,13 +354,16 @@ static int apparmor_path_link(struct dentry *old_dentry, const struct path *new_
 }
 
 static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_dentry,
-				const struct path *new_dir, struct dentry *new_dentry)
+				const struct path *new_dir, struct dentry *new_dentry,
+				const unsigned int flags)
 {
 	struct aa_label *label;
 	int error = 0;
 
 	if (!path_mediated_fs(old_dentry))
 		return 0;
+	if ((flags & RENAME_EXCHANGE) && !path_mediated_fs(new_dentry))
+		return 0;
 
 	label = begin_current_label_crit_section();
 	if (!unconfined(label)) {
@@ -374,10 +377,27 @@ static int apparmor_path_rename(const struct path *old_dir, struct dentry *old_d
 			d_backing_inode(old_dentry)->i_mode
 		};
 
-		error = aa_path_perm(OP_RENAME_SRC, label, &old_path, 0,
-				     MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
-				     AA_MAY_SETATTR | AA_MAY_DELETE,
-				     &cond);
+		if (flags & RENAME_EXCHANGE) {
+			struct path_cond cond_exchange = {
+				i_uid_into_mnt(mnt_userns, d_backing_inode(new_dentry)),
+				d_backing_inode(new_dentry)->i_mode
+			};
+
+			error = aa_path_perm(OP_RENAME_SRC, label, &new_path, 0,
+					     MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
+					     AA_MAY_SETATTR | AA_MAY_DELETE,
+					     &cond_exchange);
+			if (!error)
+				error = aa_path_perm(OP_RENAME_DEST, label, &old_path,
+						     0, MAY_WRITE | AA_MAY_SETATTR |
+						     AA_MAY_CREATE, &cond_exchange);
+		}
+
+		if (!error)
+			error = aa_path_perm(OP_RENAME_SRC, label, &old_path, 0,
+					     MAY_READ | AA_MAY_GETATTR | MAY_WRITE |
+					     AA_MAY_SETATTR | AA_MAY_DELETE,
+					     &cond);
 		if (!error)
 			error = aa_path_perm(OP_RENAME_DEST, label, &new_path,
 					     0, MAY_WRITE | AA_MAY_SETATTR |
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 7b7860039a08..30b42cdee52e 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -622,10 +622,12 @@ static int hook_path_link(struct dentry *const old_dentry,
 static int hook_path_rename(const struct path *const old_dir,
 			    struct dentry *const old_dentry,
 			    const struct path *const new_dir,
-			    struct dentry *const new_dentry)
+			    struct dentry *const new_dentry,
+			    const unsigned int flags)
 {
 	const struct landlock_ruleset *const dom =
 		landlock_get_current_domain();
+	u32 exchange_access = 0;
 
 	if (!dom)
 		return 0;
@@ -633,12 +635,19 @@ static int hook_path_rename(const struct path *const old_dir,
 	if (old_dir->dentry != new_dir->dentry)
 		/* Gracefully forbids reparenting. */
 		return -EXDEV;
+	if (flags & RENAME_EXCHANGE) {
+		if (unlikely(d_is_negative(new_dentry)))
+			return -ENOENT;
+		exchange_access =
+			get_mode_access(d_backing_inode(new_dentry)->i_mode);
+	}
 	if (unlikely(d_is_negative(old_dentry)))
 		return -ENOENT;
 	/* RENAME_EXCHANGE is handled because directories are the same. */
 	return check_access_path(
 		dom, old_dir,
 		maybe_remove(old_dentry) | maybe_remove(new_dentry) |
+			exchange_access |
 			get_mode_access(d_backing_inode(old_dentry)->i_mode));
 }
 
diff --git a/security/security.c b/security/security.c
index b7cf5cbfdc67..c9bfc0b70b28 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1197,15 +1197,8 @@ int security_path_rename(const struct path *old_dir, struct dentry *old_dentry,
 		     (d_is_positive(new_dentry) && IS_PRIVATE(d_backing_inode(new_dentry)))))
 		return 0;
 
-	if (flags & RENAME_EXCHANGE) {
-		int err = call_int_hook(path_rename, 0, new_dir, new_dentry,
-					old_dir, old_dentry);
-		if (err)
-			return err;
-	}
-
 	return call_int_hook(path_rename, 0, old_dir, old_dentry, new_dir,
-				new_dentry);
+				new_dentry, flags);
 }
 EXPORT_SYMBOL(security_path_rename);
 
diff --git a/security/tomoyo/tomoyo.c b/security/tomoyo/tomoyo.c
index b6a31901f289..71e82d855ebf 100644
--- a/security/tomoyo/tomoyo.c
+++ b/security/tomoyo/tomoyo.c
@@ -264,17 +264,26 @@ static int tomoyo_path_link(struct dentry *old_dentry, const struct path *new_di
  * @old_dentry: Pointer to "struct dentry".
  * @new_parent: Pointer to "struct path".
  * @new_dentry: Pointer to "struct dentry".
+ * @flags: Rename options.
  *
  * Returns 0 on success, negative value otherwise.
  */
 static int tomoyo_path_rename(const struct path *old_parent,
 			      struct dentry *old_dentry,
 			      const struct path *new_parent,
-			      struct dentry *new_dentry)
+			      struct dentry *new_dentry,
+			      const unsigned int flags)
 {
 	struct path path1 = { .mnt = old_parent->mnt, .dentry = old_dentry };
 	struct path path2 = { .mnt = new_parent->mnt, .dentry = new_dentry };
 
+	if (flags & RENAME_EXCHANGE) {
+		const int err = tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path2,
+				&path1);
+
+		if (err)
+			return err;
+	}
 	return tomoyo_path2_perm(TOMOYO_TYPE_RENAME, &path1, &path2);
 }
 
-- 
2.35.1


  parent reply	other threads:[~2022-05-06 16:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-06 16:10 [PATCH v3 00/12] Landlock: file linking and renaming support Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 01/12] landlock: Define access_mask_t to enforce a consistent access mask size Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 02/12] landlock: Reduce the maximum number of layers to 16 Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 03/12] landlock: Create find_rule() from unmask_layers() Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 04/12] landlock: Fix same-layer rule unions Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 05/12] landlock: Move filesystem helpers and add a new one Mickaël Salaün
2022-05-06 16:10 ` Mickaël Salaün [this message]
2022-05-06 16:10 ` [PATCH v3 07/12] landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 08/12] selftests/landlock: Add 11 new test suites dedicated to file reparenting Mickaël Salaün
2022-05-06 16:10 ` [PATCH v3 09/12] samples/landlock: Add support for " Mickaël Salaün
2022-05-06 16:11 ` [PATCH v3 10/12] landlock: Document LANDLOCK_ACCESS_FS_REFER and ABI versioning Mickaël Salaün
2022-05-06 16:11 ` [PATCH v3 11/12] landlock: Document good practices about filesystem policies Mickaël Salaün
2022-05-06 16:11 ` [PATCH v3 12/12] landlock: Add design choices documentation for filesystem access rights Mickaël Salaün
2022-05-06 16:31 ` [PATCH v3 00/12] Landlock: file linking and renaming support Mickaël Salaün

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220506161102.525323-7-mic@digikod.net \
    --to=mic@digikod.net \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=john.johansen@canonical.com \
    --cc=keescook@chromium.org \
    --cc=konstantin.meskhidze@huawei.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    --cc=takedakn@nttdata.co.jp \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH v3 06/12] LSM: Remove double path_rename hook calls for RENAME_EXCHANGE' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.