linux-kernel - Re: [PATCH] (2.6.24-rc3-mm2) -mm Smack mutex cleanup

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] [day] [month] [year] [list]

Message-ID: <267816.97172.qm@web36601.mail.mud.yahoo.com>
Date:	Mon, 3 Dec 2007 21:04:34 -0800 (PST)
From:	Casey Schaufler <casey@...aufler-ca.com>
To:	Jiri Slaby <jirislaby@...il.com>, casey@...aufler-ca.com
Cc:	akpm@...l.org, torvalds@...l.org, linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org
Subject: Re: [PATCH] (2.6.24-rc3-mm2) -mm Smack mutex cleanup


--- Jiri Slaby <jirislaby@...il.com> wrote:

> On 12/03/2007 07:39 PM, Casey Schaufler wrote:
> > From: Casey Schaufler <casey@...aufler-ca.com>
> > 
> > Clean out unnecessary mutex initializations for Smack list locks.
> > Once this is done, there is no need for them to be shared among
> > multiple files, so pull them out of the header file and put them
> > in the files where they belong.
> 
> Then it might be static.

Doh. Right you are.
 
> > Pull unnecessary locking from smack_inode_setsecurity, it used
> > to be required when the assignment was not guaranteed to be a
> > scalar value but isn't now.
> > 
> > Change uses of __capable(current,...) to capable(...).
> > Take out an inappropriate cast. Use container_of() instead
> > of doing the same calculation by hand.
> > Fix comment spelling errors.
> 
> Too many different changes according to the name of the patch.

OK, that's fair.
 
> > Signed-off-by: Casey Schaufler <casey@...aufler-ca.com>
> > 
> > ---
> > 
> > Tested with stamp-2007-11-30-16-39
> > 
> >  security/smack/smack.h        |    3 --
> >  security/smack/smack_access.c |    3 ++
> >  security/smack/smack_lsm.c    |   34 +++++++++-----------------------
> >  security/smack/smackfs.c      |    6 +++++
> >  4 files changed, 19 insertions(+), 27 deletions(-)
> > 
> > diff -uprN -X linux-2.6.24-rc3-mm2-base/Documentation/dontdiff
> linux-2.6.24-rc3-mm2-base/security/smack/smack_lsm.c
> linux-2.6.24-rc3-mm2-smack/security/smack/smack_lsm.c
> > --- linux-2.6.24-rc3-mm2-base/security/smack/smack_lsm.c	2007-11-27
> 16:47:05.000000000 -0800
> > +++ linux-2.6.24-rc3-mm2-smack/security/smack/smack_lsm.c	2007-11-28
> 11:46:13.000000000 -0800
> [...]
> > @@ -748,9 +746,7 @@ static int smack_inode_setsecurity(struc
> >  		return -EINVAL;
> >  
> >  	if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
> > -		mutex_lock(&nsp->smk_lock);
> >  		nsp->smk_inode = sp;
> > -		mutex_unlock(&nsp->smk_lock);
> >  		return 0;
> >  	}
> >  	/*
> 
> Ok, it still might be atomic as a variable change, but it will break
> scenarios
> such as
> 
> mutex_lock(&nsp->smk_lock);
> create(nsp->smk_inode);
> cook_a_dinner();
> get_info(nsp->smk_inode);
> mutex_unlock(&nsp->smk_lock);
> 
> While cook_a_dinner(), smack_inode_setsecurity() is called and the attribute
> changed...
> 
> Doesn't this matter?

The only place dinner can get cooked is during d_instantiate, and
you can't call inode_security until after that's finished. No,
it doesn't matter.


Casey Schaufler
casey@...aufler-ca.com
--
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/