Message-ID: <20041110174358.32392.qmail@lwn.net>
To: Serge Hallyn <serue@us.ibm.com>
Cc: Andrew Morton <akpm@osdl.org>, lkml <linux-kernel@vger.kernel.org>,
       Chris Wright <chrisw@osdl.org>
Subject: Re: [RFC] [PATCH] [2/6] LSM Stacking: Add stacker LSM 
From: corbet@lwn.net (Jonathan Corbet)
In-reply-to: Your message of "Thu, 04 Nov 2004 17:08:01 CST."
             <1099609681.2096.16.camel@serge.austin.ibm.com> 
Date: Wed, 10 Nov 2004 10:43:58 -0700
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1556
Lines: 45

Without addressing the question of whether stacking modules makes sense
in the first place, I'd like to note a couple of things which caught my
eye:

> +static int stacker_register (const char *name, struct
> security_operations *ops)
> +{
> +	/* This function is the primary reason for the stacker module.
> +	   Add the stacked module (as specified by name and ops)
> +	   according to the current ordering policy. */
> +
> +	char *new_module_name;
> +	struct module_entry *new_module_entry;
> +	int namelen;
> +
> +	num_stacked_modules++;
> [...]
> +	return num_stacked_modules-1;
> +}

Unless I've missed it, you never check num_stacked_modules against
CONFIG_NUM_LSMS.  If somebody loads too many modules, they risk
overflowing all of those void * security arrays you've added to so many
kernel data structures, and thus corrupting those structures.  That, in
technical terms, would be a bummer.

In stacker_unregister(), you do:

> +	num_stacked_modules--;

What happens if you unload anything other than the last module, then
load something else?  When you return num_stacked_modules-1 to the new
module, you'll point it to a slot in those security arrays which is
already used by another module.  The result seems unlikely to improve
security.

Unless I'm simply confused?  It's happened before...

jon

-
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/