diff mbox series

[RFC,1/3] apparmor: use SB_* flags for private sb flags

Message ID 20230110022554.1186499-2-mcgrof@kernel.org (mailing list archive)
State New, archived
Headers show
Series fs: kill old ms_* flags for internal sb | expand

Commit Message

Luis Chamberlain Jan. 10, 2023, 2:25 a.m. UTC
Commit 2ea3ffb7782 ("apparmor: add mount mediation") John Johansen
added mount mediation support. However just the day before this commit
David Howells modified the internal sb flags through commit e462ec50cb5
("VFS: Differentiate mount flags (MS_*) from internal superblock flags").

Use the modified sb flags to make things clear and avoid further uses
of the old MS_* flags for superblock internal flags. This will let us
later remove the MS_* sb internal flags as userspace should not be
using them.

This commit does not fix anything as the old flags used map to the
same bitmask, this just tidies things up. I split up the flags to
make it clearer which ones are for the superblock and used internally.

Cc: John Johansen <john.johansen@canonical.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
---
 security/apparmor/include/mount.h | 3 ++-
 security/apparmor/lsm.c           | 1 +
 security/apparmor/mount.c         | 2 +-
 3 files changed, 4 insertions(+), 2 deletions(-)

Comments

Al Viro Jan. 10, 2023, 11:40 p.m. UTC | #1
On Mon, Jan 09, 2023 at 06:25:52PM -0800, Luis Chamberlain wrote:
> Commit 2ea3ffb7782 ("apparmor: add mount mediation") John Johansen
> added mount mediation support. However just the day before this commit
> David Howells modified the internal sb flags through commit e462ec50cb5
> ("VFS: Differentiate mount flags (MS_*) from internal superblock flags").
> 
> Use the modified sb flags to make things clear and avoid further uses
> of the old MS_* flags for superblock internal flags. This will let us
> later remove the MS_* sb internal flags as userspace should not be
> using them.
> 
> This commit does not fix anything as the old flags used map to the
> same bitmask, this just tidies things up. I split up the flags to
> make it clearer which ones are for the superblock and used internally.

I don't think that's right.  apparmor_sb_mount() gets (almost) raw flags
from mount(2); incidentally, MS_MGC_MSK removal directly above the modified
line is BS since _that_ has already been done by the caller.

Note that the same function explicitly checks for MS_MOVE, etc. in the
same argument.

> @@ -74,7 +74,7 @@ static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
>  		audit_log_format(ab, ", iversion");
>  	if (flags & MS_STRICTATIME)
>  		audit_log_format(ab, ", strictatime");
> -	if (flags & MS_NOUSER)
> +	if (flags & SB_NOUSER)
>  		audit_log_format(ab, ", nouser");
>  }

Umm...  How does one trigger that one?
diff mbox series

Patch

diff --git a/security/apparmor/include/mount.h b/security/apparmor/include/mount.h
index a710683b2496..f90e03405e38 100644
--- a/security/apparmor/include/mount.h
+++ b/security/apparmor/include/mount.h
@@ -23,7 +23,8 @@ 
 #define AA_AUDIT_DATA		0x40
 #define AA_MNT_CONT_MATCH	0x40
 
-#define AA_MS_IGNORE_MASK (MS_KERNMOUNT | MS_NOSEC | MS_ACTIVE | MS_BORN)
+#define AA_MS_IGNORE_MASK (MS_KERNMOUNT)
+#define AA_SB_IGNORE_MASK (SB_NOSEC | SB_ACTIVE | SB_BORN)
 
 int aa_remount(struct aa_label *label, const struct path *path,
 	       unsigned long flags, void *data);
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index c6728a629437..f3880956bffd 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -583,6 +583,7 @@  static int apparmor_sb_mount(const char *dev_name, const struct path *path,
 		flags &= ~MS_MGC_MSK;
 
 	flags &= ~AA_MS_IGNORE_MASK;
+	flags &= ~AA_SB_IGNORE_MASK;
 
 	label = __begin_current_label_crit_section();
 	if (!unconfined(label)) {
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index cdfa430ae216..c37c451e8226 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -74,7 +74,7 @@  static void audit_mnt_flags(struct audit_buffer *ab, unsigned long flags)
 		audit_log_format(ab, ", iversion");
 	if (flags & MS_STRICTATIME)
 		audit_log_format(ab, ", strictatime");
-	if (flags & MS_NOUSER)
+	if (flags & SB_NOUSER)
 		audit_log_format(ab, ", nouser");
 }