public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ifdef struct task_struct::security
@ 2007-08-06 18:55 Alexey Dobriyan
  2007-08-06 20:31 ` Serge E. Hallyn
  0 siblings, 1 reply; 8+ messages in thread
From: Alexey Dobriyan @ 2007-08-06 18:55 UTC (permalink / raw)
  To: akpm; +Cc: linux-kernel

For those who don't care about CONFIG_SECURITY.

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;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-06 18:55 [PATCH] ifdef struct task_struct::security Alexey Dobriyan
@ 2007-08-06 20:31 ` Serge E. Hallyn
  2007-08-07  5:08   ` Andrew Morton
  0 siblings, 1 reply; 8+ messages in thread
From: Serge E. Hallyn @ 2007-08-06 20:31 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: akpm, linux-kernel

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.

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-06 20:31 ` Serge E. Hallyn
@ 2007-08-07  5:08   ` Andrew Morton
  2007-08-07 15:05     ` Serge E. Hallyn
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2007-08-07  5:08 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: Alexey Dobriyan, linux-kernel

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.

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



In fact, looking through all those "= 0" and "= NULL" statements in
copy_process() makes one wonder whether we should be memsetting that guy to
zero then selectively copying things out of current, rather than the
present vice-versa.

A possibly-neat way of doing this would be to move all the task_struct fields which
are zeroed in copy_process() into a separate anonymous struct in
task_struct, then wipe only that in copy_process().  One would need to be
careful about the hand-arranged grouping which has been done in the
task_struct however.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-07  5:08   ` Andrew Morton
@ 2007-08-07 15:05     ` Serge E. Hallyn
  2007-08-07 15:57       ` Casey Schaufler
  2007-08-07 19:04       ` Alexey Dobriyan
  0 siblings, 2 replies; 8+ messages in thread
From: Serge E. Hallyn @ 2007-08-07 15:05 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Serge E. Hallyn, Alexey Dobriyan, linux-kernel

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)

Using CONFIG_SECURITY means that if you compile with SECURITY=n, you get
the capability module but no task->security.  If you compile with
SECURITY=y but no modules, you get the dummy module and a
task->security field!

> In fact, looking through all those "= 0" and "= NULL" statements in
> copy_process() makes one wonder whether we should be memsetting that guy to
> zero then selectively copying things out of current, rather than the
> present vice-versa.
> 
> A possibly-neat way of doing this would be to move all the task_struct fields which
> are zeroed in copy_process() into a separate anonymous struct in
> task_struct, then wipe only that in copy_process().  One would need to be
> careful about the hand-arranged grouping which has been done in the
> task_struct however.

thanks,
-serge

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-07 15:05     ` Serge E. Hallyn
@ 2007-08-07 15:57       ` Casey Schaufler
  2007-08-07 16:12         ` Serge E. Hallyn
  2007-08-07 19:04       ` Alexey Dobriyan
  1 sibling, 1 reply; 8+ messages in thread
From: Casey Schaufler @ 2007-08-07 15:57 UTC (permalink / raw)
  To: Serge E. Hallyn, Andrew Morton
  Cc: Serge E. Hallyn, Alexey Dobriyan, linux-kernel


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-07 15:57       ` Casey Schaufler
@ 2007-08-07 16:12         ` Serge E. Hallyn
  2007-08-08  0:34           ` Andrew Morton
  0 siblings, 1 reply; 8+ messages in thread
From: Serge E. Hallyn @ 2007-08-07 16:12 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Serge E. Hallyn, Andrew Morton, Alexey Dobriyan, linux-kernel

Quoting 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.

So jump straight to using CONFIG_SECURITY_USE_LABELS or whatever, as I
mentioned.

-serge

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-07 15:05     ` Serge E. Hallyn
  2007-08-07 15:57       ` Casey Schaufler
@ 2007-08-07 19:04       ` Alexey Dobriyan
  1 sibling, 0 replies; 8+ messages in thread
From: Alexey Dobriyan @ 2007-08-07 19:04 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: Andrew Morton, linux-kernel

On Tue, Aug 07, 2007 at 10:05:29AM -0500, Serge E. Hallyn 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)
> 
> Using CONFIG_SECURITY means that if you compile with SECURITY=n, you get
> the capability module but no task->security.  If you compile with
> SECURITY=y but no modules, you get the dummy module and a
> task->security field!

If I understood intent correctly CONFIG_SECURITY_ATTRS will be an overkill
because of one more compilation breaking option and small amount of
people benefitting from it.

How much people have such setup? Example: for more than 4 years nobody from
CONFIG_SECURITY=n camp cared about their inodes and struct files being bigger
than needed. Even more time for task_struct and fork being slower.

> > In fact, looking through all those "= 0" and "= NULL" statements in
> > copy_process() makes one wonder whether we should be memsetting that guy to
> > zero then selectively copying things out of current, rather than the
> > present vice-versa.
> > 
> > A possibly-neat way of doing this would be to move all the task_struct fields which
> > are zeroed in copy_process() into a separate anonymous struct in
> > task_struct, then wipe only that in copy_process().  One would need to be
> > careful about the hand-arranged grouping which has been done in the
> > task_struct however.

Interesting... I am sure this was tried in good old times when task_struct
was not so bloated, maybe now it will be net win now.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ifdef struct task_struct::security
  2007-08-07 16:12         ` Serge E. Hallyn
@ 2007-08-08  0:34           ` Andrew Morton
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2007-08-08  0:34 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: Casey Schaufler, Alexey Dobriyan, linux-kernel

On Tue, 7 Aug 2007 11:12:02 -0500
"Serge E. Hallyn" <serge@hallyn.com> wrote:

> Quoting 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.
> 
> So jump straight to using CONFIG_SECURITY_USE_LABELS or whatever, as I
> mentioned.
> 

Well I've lost the plot here, but I'm all for shrinking task_struct on
small systems, so I'll trollmerge Alexey's original diff.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2007-08-08  0:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-06 18:55 [PATCH] ifdef struct task_struct::security Alexey Dobriyan
2007-08-06 20:31 ` Serge E. Hallyn
2007-08-07  5:08   ` Andrew Morton
2007-08-07 15:05     ` Serge E. Hallyn
2007-08-07 15:57       ` Casey Schaufler
2007-08-07 16:12         ` Serge E. Hallyn
2007-08-08  0:34           ` Andrew Morton
2007-08-07 19:04       ` Alexey Dobriyan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox