Re: [PATCH] ifdef struct task_struct::security

[Casey Schaufler <casey@schaufler-ca.com>]

--- "Serge E. Hallyn" <serge@hallyn.com> wrote:

> Quoting Andrew Morton (akpm@linux-foundation.org):
> > On Mon, 6 Aug 2007 15:31:12 -0500 "Serge E. Hallyn" <serge@hallyn.com>
> wrote:
> > 
> > > Quoting Alexey Dobriyan (adobriyan@gmail.com):
> > > > For those who don't care about CONFIG_SECURITY.
> > > 
> > > I'm quite sure we started that way, but the ifdefs were considered
> > > too much of an eyesore.
> > 
> > argh, y'all stop top-posting at me.
> 
> (Hmm, I'm replying at the point in the email I'm replying to.  Is what
> I'm doing in this current email ok - i.e the one you replied to looked
> like pure top-posting - or do you actually want pure bottom posting?)
> 
> > > If this is now acceptable, then the same thing might be considered
> > > for inode->i_security, kern_ipc_perm.security, etc.  Getting rid of
> > > just the task->security seems overly half-hearted.
> > > 
> > > -serge
> > > 
> > > > Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
> > > > ---
> > > > 
> > > >  include/linux/sched.h |    3 ++-
> > > >  kernel/fork.c         |    2 ++
> > > >  2 files changed, 4 insertions(+), 1 deletion(-)
> > > > 
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -1086,8 +1086,9 @@ struct task_struct {
> > > >  	int (*notifier)(void *priv);
> > > >  	void *notifier_data;
> > > >  	sigset_t *notifier_mask;
> > > > -	
> > > > +#ifdef CONFIG_SECURITY
> > > >  	void *security;
> > > > +#endif
> > > >  	struct audit_context *audit_context;
> > > >  	seccomp_t seccomp;
> > > >  
> > > > --- a/kernel/fork.c
> > > > +++ b/kernel/fork.c
> > > > @@ -1066,7 +1066,9 @@ static struct task_struct *copy_process(unsigned
> long clone_flags,
> > > >  	do_posix_clock_monotonic_gettime(&p->start_time);
> > > >  	p->real_start_time = p->start_time;
> > > >  	monotonic_to_bootbased(&p->real_start_time);
> > > > +#ifdef CONFIG_SECURITY
> > > >  	p->security = NULL;
> > > > +#endif
> > > >  	p->io_context = NULL;
> > > >  	p->io_wait = NULL;
> > > >  	p->audit_context = NULL;
> > > > 
> > 
> > I think it's OK.  Removing 4 or 8 bytes from the task_struct is a decent
> win,
> > and an ifdef at the definition site (unavoidable) and at a single
> > initialisation site where there are lots of other similar ifdefs is pretty
> > minimal hurt.
> 
> Then how about making it depend on CONFIG_SECURITY_SELINUX?  It's the
> only LSM actually using that field right now.  (As more come along, we
> can use a hidden CONFIG_SECURITY_ATTRS or somesuch bool select'ed by
> LSMs which need it)

I would greatly appreciate it if you didn't add yet another place
that requires deselinuxifation by anyone wanting to try something else.
The question is whether there is any LSM, not whether there is selinux.
Yes, I know that there are no other LSMs upstream today. I hope to
change that before too long, and dealing with places where the code is
using the LSM==SELinux assumption is tiresome.


Casey Schaufler
casey@schaufler-ca.com