LKML Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
@ 2008-03-04 17:45 Casey Schaufler
  2008-03-04 18:12 ` Linus Torvalds
  2008-03-05 12:44 ` Ahmed S. Darwish
  0 siblings, 2 replies; 10+ messages in thread
From: Casey Schaufler @ 2008-03-04 17:45 UTC (permalink / raw)
  To: Linus Torvalds, Ahmed S. Darwish; +Cc: LKML

----- Original Message ----
> From: Linus Torvalds <torvalds@linux-foundation.org>
> To: Ahmed S. Darwish <darwish.07@gmail.com>
> Cc: Casey Schaufler <casey@schaufler-ca.com>; LKML <linux-kernel@vger.kernel.org>
> Sent: Tuesday, March 4, 2008 9:21:19 AM
> Subject: Re: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
> 
> 
> 
> On Tue, 4 Mar 2008, Ahmed S. Darwish wrote:
> > 
> > Smackfs initialization without an enabled Smack leads to
> > an early Oops that renders the system unusable.
> 
> I really think this is bogus. Global enables like this are just wrong, and 
> a sign that something else bad is going on.
> 
> What is the oops? Why does it happen?

A kernel that is built with both SELinux and Smack contains
all of the components for both, including smackfs. If SELinux
is chosen as the module to be used and smackfs is initialized
the oops occurs because the Smack initialization that smackfs
depends on has not been done.

One solution would be to tighten the smackfs code so that it
handles the uninitialized LSM case properly.

Another would be to set up Kconfig as to make SELinux and Smack
mutually exclusive, although I really don't know how well that
would go over in testing circles because "config all on" becomes
ambiguous.

A third would be to provide for stacking, but I assume that's
beyond the scope of this exercise.

So I think that fixing up smackfs is the right choice at this point.

> 
> 
> Linus



 
Casey Schaufler
casey@schaufler-ca.com 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
@ 2008-03-04 16:42 Casey Schaufler
  0 siblings, 0 replies; 10+ messages in thread
From: Casey Schaufler @ 2008-03-04 16:42 UTC (permalink / raw)
  To: Ahmed S. Darwish; +Cc: LKML, Linus


----- Original Message ----
> From: Ahmed S. Darwish <darwish.07@gmail.com>
> To: Casey Schaufler <casey@schaufler-ca.com>
> Cc: LKML <linux-kernel@vger.kernel.org>; Linus <torvalds@linux-foundation.org>
> Sent: Tuesday, March 4, 2008 5:10:55 AM
> Subject: [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
> 
> Hi all,
> 
> Smackfs initialization without an enabled Smack leads to
> an early Oops that renders the system unusable.
> 
> Introduce a global smack_enabled variable that will be used
> to make sure that no smack components will be registered 
> (ala smackfs) if we are not already enabled.
> 
> Signed-off-by: Ahmed S. Darwish
Acked-by: Casey Schaufler casey@schaufler-ca.com

> ---
> 
> The Oops is triggered by the security= patch that will be sent
> soon.
> 
> I can't imagine an SELinux guru finding /smackfs instead of 
> his usual /selinuxfs when he hits a tab completion after 's' ;).
> As a bonus, this patch will handle that case too.
> 
> smack.h | 9 +++++++++
> smack_lsm.c | 8 ++++++++
> smackfs.c | 3 +++
> 
> 3 files changed, 20 insertions(+)
> 
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index a21a0e9..17c55ad 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -18,6 +18,15 @@
> #include 
> 
> /*
> + * We must not bother the rest of the kernel by exporting our
> + * own stuff if we are not already enabled. We may not be loaded
> + * if another or no LSM was chosen on boot.
> + * Smackfs is currently the only exported component, but this
> + * may change in the future.
> + */
> +extern int smack_enabled;
> +
> +/*
> * Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
> * bigger than can be used, and 24 is the next lower multiple
> * of 8, and there are too many issues if there isn't space set
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 770eb06..6fe7869 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -36,6 +36,8 @@
> #define SOCKFS_MAGIC 0x534F434B
> #define TMPFS_MAGIC 0x01021994
> 
> +int smack_enabled;
> +
> /**
> * smk_fetch - Fetch the smack label from a file.
> * @ip: a pointer to the inode
> @@ -2589,6 +2591,12 @@ static __init int smack_init(void)
> if (register_security(&smack_ops))
> panic("smack: Unable to register with kernel.\n");
> 
> + /*
> + * Notify other Smack components that it's now safe to
> + * to register themselves.
> + */
> + smack_enabled = 1;
> +
> return 0;
> }
> 
> diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
> index 358c92c..e1687c0 100644
> --- a/security/smack/smackfs.c
> +++ b/security/smack/smackfs.c
> @@ -992,6 +992,9 @@ static int __init init_smk_fs(void)
> {
> int err;
> 
> + if (!smack_enabled)
> + return 0;
> +
> err = register_filesystem(&smk_fs_type);
> if (!err) {
> smackfs_mount = kern_mount(&smk_fs_type);
> 
> -- 
> 
> "Better to light a candle, than curse the darkness"
> 
> Ahmed S. Darwish
> Homepage: http://darwish.07.googlepages.com
> Blog: http://darwish-07.blogspot.com
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at 
> http://www.tux.org/lkml/


Casey Schaufler
casey@schaufler-ca.com 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded
@ 2008-03-04 13:10 Ahmed S. Darwish
  2008-03-04 17:21 ` Linus Torvalds
  0 siblings, 1 reply; 10+ messages in thread
From: Ahmed S. Darwish @ 2008-03-04 13:10 UTC (permalink / raw)
  To: Casey Schaufler; +Cc: LKML, Linus

Hi all,

Smackfs initialization without an enabled Smack leads to
an early Oops that renders the system unusable.

Introduce a global smack_enabled variable that will be used
to make sure that no smack components will be registered 
(ala smackfs) if we are not already enabled.

Signed-off-by: Ahmed S. Darwish <darwish.07@gmail.com>
---

The Oops is triggered by the security= patch that will be sent
soon.

I can't imagine an SELinux guru finding /smackfs instead of 
his usual /selinuxfs when he hits a tab completion after 's' ;).
As a bonus, this patch will handle that case too.

 smack.h     |    9 +++++++++
 smack_lsm.c |    8 ++++++++
 smackfs.c   |    3 +++

 3 files changed, 20 insertions(+)

diff --git a/security/smack/smack.h b/security/smack/smack.h
index a21a0e9..17c55ad 100644
--- a/security/smack/smack.h
+++ b/security/smack/smack.h
@@ -18,6 +18,15 @@
 #include <net/netlabel.h>
 
 /*
+ * We must not bother the rest of the kernel by exporting our
+ * own stuff if we are not already enabled. We may not be loaded
+ * if another or no LSM was chosen on boot.
+ * Smackfs is currently the only exported component, but this
+ * may change in the future.
+ */
+extern int smack_enabled;
+
+/*
  * Why 23? CIPSO is constrained to 30, so a 32 byte buffer is
  * bigger than can be used, and 24 is the next lower multiple
  * of 8, and there are too many issues if there isn't space set
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 770eb06..6fe7869 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -36,6 +36,8 @@
 #define SOCKFS_MAGIC		0x534F434B
 #define TMPFS_MAGIC		0x01021994
 
+int smack_enabled;
+
 /**
  * smk_fetch - Fetch the smack label from a file.
  * @ip: a pointer to the inode
@@ -2589,6 +2591,12 @@ static __init int smack_init(void)
 	if (register_security(&smack_ops))
 		panic("smack: Unable to register with kernel.\n");
 
+	/*
+	 * Notify other Smack components that it's now safe to
+	 * to register themselves.
+	 */
+	smack_enabled = 1;
+
 	return 0;
 }
 
diff --git a/security/smack/smackfs.c b/security/smack/smackfs.c
index 358c92c..e1687c0 100644
--- a/security/smack/smackfs.c
+++ b/security/smack/smackfs.c
@@ -992,6 +992,9 @@ static int __init init_smk_fs(void)
 {
 	int err;
 
+	if (!smack_enabled)
+		return 0;
+
 	err = register_filesystem(&smk_fs_type);
 	if (!err) {
 		smackfs_mount = kern_mount(&smk_fs_type);

-- 

"Better to light a candle, than curse the darkness"

Ahmed S. Darwish
Homepage: http://darwish.07.googlepages.com
Blog: http://darwish-07.blogspot.com


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, back to index

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-03-04 17:45 [PATCH BUGFIX -rc3] Smack: Don't register smackfs if we're not loaded Casey Schaufler
2008-03-04 18:12 ` Linus Torvalds
2008-03-05  0:58   ` James Morris
2008-03-05 12:12     ` Ahmed S. Darwish
2008-03-05 12:44 ` Ahmed S. Darwish
2008-03-05 12:51   ` Ahmed S. Darwish
  -- strict thread matches above, loose matches on Subject: below --
2008-03-04 16:42 Casey Schaufler
2008-03-04 13:10 Ahmed S. Darwish
2008-03-04 17:21 ` Linus Torvalds
2008-03-04 18:24   ` Ahmed S. Darwish

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lkml.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lkml.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lkml.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lkml.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lkml.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lkml.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lkml.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lkml.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lkml.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lkml.kernel.org/lkml/9 lkml/git/9.git
	git clone --mirror https://lkml.kernel.org/lkml/10 lkml/git/10.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lkml.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git