linux-kernel - Re: [PATCH] Security/sysfs: Enable security xattrs to be set on sysfs files, directories, and symlinks.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1250510482.3629.97.camel@moss-pluto.epoch.ncsc.mil>
Date:	Mon, 17 Aug 2009 08:01:22 -0400
From:	Stephen Smalley <sds@...ho.nsa.gov>
To:	Casey Schaufler <casey@...aufler-ca.com>
Cc:	"David P. Quigley" <dpquigl@...ho.nsa.gov>, jmorris@...ei.org,
	Greg Kroah-Hartman <greg@...ah.com>, ebiederm@...ssion.com,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, selinux@...ho.nsa.gov
Subject: Re: [PATCH] Security/sysfs: Enable security xattrs to be set on
 sysfs files, directories, and symlinks.

On Fri, 2009-08-14 at 18:33 -0700, Casey Schaufler wrote:
> Stephen Smalley wrote:
> > On Fri, 2009-08-14 at 08:20 -0400, Stephen Smalley wrote:
> >   
> >> ...
> >>> + */
> >>> +static DEFINE_MUTEX(sysfs_xattr_lock);
> >>> +
> >>> +static struct sysfs_xattr *new_xattr(const char *name, const void *value,
> >>> +					size_t size)
> >>> +{
> >>> +	struct sysfs_xattr *nxattr;
> >>> +	void *nvalue;
> >>> +	char *nname;
> >>> +
> >>> +	nxattr = kzalloc(sizeof(*nxattr), GFP_KERNEL);
> >>> +	if (!nxattr)
> >>> +		return NULL;
> >>> +	nvalue = kzalloc(size, GFP_KERNEL);
> >>> +	if (!nvalue) {
> >>> +		kfree(nxattr);
> >>> +		return NULL;
> >>> +	}
> >>> +	nname = kzalloc(strlen(name) + 1, GFP_KERNEL);
> >>> +	if (!nname) {
> >>> +		kfree(nxattr);
> >>> +		kfree(nvalue);
> >>> +		return NULL;
> >>> +	}
> >>> +	memcpy(nvalue, value, size);
> >>> +	strcpy(nname, name);
> >>> +	nxattr->sx_name = nname;
> >>> +	nxattr->sx_value = nvalue;
> >>> +	nxattr->sx_size = size;
> >>>       
> >> Storing the name/value pairs here is redundant - the security module
> >> already has to store the value in some form (potentially smaller, like a
> >> secid + struct in the SELinux case).  This wastes memory.
> >>     
> >
> > Sorry - to clarify, I understand that we have to store a representation
> > of the security attribute in the backing data structure so that it can
> > be restored later, but that representation should come from the security
> > module rather than being the original (name, value, size) triple.  Which
> > is what David's patch does - he obtains a secid from the security module
> > for storage in the wrapped iattr structure.
> >   
> 
> Sorry, but I disagree with your assertion. An LSM can do what
> it likes with the xattr, but the value sent from userland is
> what should be stored.

Then you will definitely end up using more memory than David's approach,
as in the Smack case you'll duplicate storage of the text string by both
the filesystem and by the security module, and in the SELinux case the
filesystem will store the full text string and SELinux will store the
struct representation (full string representation is generated on
demand).

-- 
Stephen Smalley
National Security Agency

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/