lkml.org 
[lkml]   [2007]   [Aug]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] ifdef struct task_struct::security
    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.

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

    \
     
     \ /
      Last update: 2007-08-07 21:07    [W:0.030 / U:29.964 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site