linux-kernel - Re: [PATCH] selinux: init target class when add avc callback

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: <1331080893.6253.1.camel@localhost>
Date:	Tue, 06 Mar 2012 19:41:33 -0500
From:	Eric Paris <eparis@...isplace.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	gaowanlong@...fujitsu.com, linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org, James Morris <jmorris@...ei.org>,
	sds@...ho.nsa.gov
Subject: Re: [PATCH] selinux: init target class when add avc callback

On Tue, 2012-03-06 at 16:15 -0800, Andrew Morton wrote:
> On Wed, 07 Mar 2012 07:59:30 +0800
> Wanlong Gao <gaowanlong@...fujitsu.com> wrote:
> 
> > On 02/05/2012 09:53 AM, Wanlong Gao wrote:
> > 
> > > Target security class should be initialized when add avc callback.
> > > Although tclass is userless in callbacks now, but it may be used
> > > in the future .
> > > 
> > > Signed-off-by: Wanlong Gao <gaowanlong@...fujitsu.com>
> > > ---
> > >  security/selinux/avc.c |    1 +
> > >  1 files changed, 1 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/security/selinux/avc.c b/security/selinux/avc.c
> > > index dca1c22..27495e6 100644
> > > --- a/security/selinux/avc.c
> > > +++ b/security/selinux/avc.c
> > > @@ -576,6 +576,7 @@ int avc_add_callback(int (*callback)(u32 event, u32 ssid, u32 tsid,
> > >  	c->events = events;
> > >  	c->ssid = ssid;
> > >  	c->tsid = tsid;
> > > +	c->tclass = tclass;
> > >  	c->perms = perms;
> > >  	c->next = avc_callbacks;
> > >  	avc_callbacks = c;
> 
> Perhaps James can take a look at this?
> 
> avc_add_callback() looks a bit odd.  It uses GFP_ATOMIC, but that is
> unnecessary because avc_add_callback() is only ever called from
> module_init() code.  And if it isn't only ever called from
> module_init() code then it needs some locking for that list.

I'm a bad maintainer.  I should have done something with this patch.
Adding sds, the only other person who ever actually maintains this code,
to the thread.

__initcall() functions aren't serialized?  I guess that would be bad and
we would need a lock.  I wonder if there are other places I assumed
__initcall() would be serialized (note that all of these call sites are
built in and not modules if that makes a difference)

I'll probably just rip all of that ssid, tsid, tclass, perms, stuff out.
If all these years noone uses callbacks for anything other than reset
why do we have it at all.  Probably more simplification we can do around
avc_update_node() too...

Stephen, thoughts on ripping stuff out?

-Eric

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