lkml.org 
[lkml]   [2006]   [Jul]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [2.6 patch] let CONFIG_SECCOMP default to n
    On Fri, Jun 30, 2006 at 11:03:00AM -0400, Lee Revell wrote:
    > On Thu, 2006-06-29 at 19:35 -0700, Randy.Dunlap wrote:
    > > On Thu, 29 Jun 2006 18:07:06 -0700 Andrew Morton wrote:
    > >
    > > > Lee Revell <rlrevell@joe-job.com> wrote:
    > > > >
    > > > > On Thu, 2006-06-29 at 21:21 +0200, Adrian Bunk wrote:
    > > > > > This patch was already sent on:
    > > > > > - 26 Jun 2006
    > > > > > - 27 Apr 2006
    > > > > > - 19 Apr 2006
    > > > > > - 11 Apr 2006
    > > > > > - 10 Mar 2006
    > > > > > - 29 Jan 2006
    > > > > > - 21 Jan 2006
    > > > >
    > > > > 3 days ago? That seems a bit silly. Why didn't you just ping Andrew on
    > > > > it?
    > > > >
    > > > > Andrew, what's the status of this? Can we get an ACK or a NACK before
    > > > > this starts getting reposted every day? ;-)
    > > > >
    > > >
    > > > I am stolidly letting the arch maintainers and the developer of this
    > > > feature work out what to do.
    > >
    > > Bah, options that are not Required should default to n.
    > > I support Adrian's patch.
    >
    > Agreed:
    >
    > - Most people don't use it
    > - There's a performance hit

    On x86-64 SECCOMP generates absoutely zero performance hit.

    The original seccomp patch for x86 also generated absolutely zero
    performance hit, both pratically and theoretically too. _zero_ CPU
    cycles of difference, zero cachelines.

    What generates a minuscle overhead is a feature I added later on top of
    SECCOMP, that disables TSC for SECCOMP tasks. I thought such minuscle
    overhead wouldn't be measurable compared to all other heavyweight work
    we do in the scheduler (note that unless you sell cpu through CPUShare
    actively this overhead consists in two cacheline touches per context
    switch), but anyway I agree it's good idea to make it optional so
    there will be absolutely no reason left to leave seccomp disabled by
    default anymore.

    Andi thinks the feature is absolutely unnecessary, he's certainly right,
    and it has been there only for paranoid reasons.

    http://www.cpushare.com/blog/CPUShare/article/26/

    > Clearly should default to N.

    I think the best is to add a CONFIG_SECCOMP_DISABLE_TSC obviously
    defaulted to N, so seccomp returns absolutely zerocost and everybody
    will be happy (me included for sure, since I agree with Andi, except I
    can't be sure of it and that's the only reason why I developed the tsc
    disable feature).

    I strongly agree with leaving CONFIG_SECCOMP_DISABLE_TSC set to N by
    default.

    -------------

    Make the TSC disable purely paranoid feature optional, so by default seccomp
    returns absolutely zerocost.

    Signed-off-by: Andrea Arcangeli <andrea@cpushare.com>

    diff -r 67137165b47d arch/i386/Kconfig
    --- a/arch/i386/Kconfig Thu Jul 06 19:45:01 2006 +0200
    +++ b/arch/i386/Kconfig Sat Jul 08 11:06:49 2006 +0200
    @@ -734,6 +734,18 @@ config SECCOMP
    defined by each seccomp mode.

    If unsure, say Y. Only embedded should say N here.
    +
    +config SECCOMP_DISABLE_TSC
    + bool "Disable the TSC for seccomp tasks"
    + depends on SECCOMP
    + default n
    + help
    + This feature mathematically prevents covert channels
    + for tasks running under SECCOMP. This can generate
    + a minuscule overhead in the scheduler.
    +
    + If you care most about performance say N. Say Y only if you're
    + paranoid about covert channels.

    source kernel/Kconfig.hz

    diff -r 67137165b47d arch/i386/kernel/process.c
    --- a/arch/i386/kernel/process.c Thu Jul 06 19:45:01 2006 +0200
    +++ b/arch/i386/kernel/process.c Sat Jul 08 11:05:35 2006 +0200
    @@ -572,6 +572,7 @@ static inline void disable_tsc(struct ta
    static inline void disable_tsc(struct task_struct *prev_p,
    struct task_struct *next_p)
    {
    +#ifdef CONFIG_SECCOMP_DISABLE_TSC
    struct thread_info *prev, *next;

    /*
    @@ -590,6 +591,7 @@ static inline void disable_tsc(struct ta
    has_secure_computing(next))
    write_cr4(read_cr4() | X86_CR4_TSD);
    }
    +#endif
    }

    /*
    diff -r 67137165b47d arch/x86_64/Kconfig
    --- a/arch/x86_64/Kconfig Thu Jul 06 19:45:01 2006 +0200
    +++ b/arch/x86_64/Kconfig Sat Jul 08 11:06:40 2006 +0200
    @@ -522,6 +522,18 @@ config SECCOMP

    If unsure, say Y. Only embedded should say N here.

    +config SECCOMP_DISABLE_TSC
    + bool "Disable the TSC for seccomp tasks"
    + depends on SECCOMP
    + default n
    + help
    + This feature mathematically prevents covert channels
    + for tasks running under SECCOMP. This can generate
    + a minuscule overhead in the scheduler.
    +
    + If you care most about performance say N. Say Y only if you're
    + paranoid about covert channels.
    +
    source kernel/Kconfig.hz

    config REORDER
    diff -r 67137165b47d arch/x86_64/kernel/process.c
    --- a/arch/x86_64/kernel/process.c Thu Jul 06 19:45:01 2006 +0200
    +++ b/arch/x86_64/kernel/process.c Sat Jul 08 11:05:26 2006 +0200
    @@ -494,6 +494,35 @@ out:
    }

    /*
    + * This function selects if the context switch from prev to next
    + * has to tweak the TSC disable bit in the cr4.
    + */
    +static inline void disable_tsc(struct task_struct *prev_p,
    + struct task_struct *next_p)
    +{
    +#ifdef CONFIG_SECCOMP_DISABLE_TSC
    + struct thread_info *prev, *next;
    +
    + /*
    + * gcc should eliminate the ->thread_info dereference if
    + * has_secure_computing returns 0 at compile time (SECCOMP=n).
    + */
    + prev = prev_p->thread_info;
    + next = next_p->thread_info;
    +
    + if (has_secure_computing(prev) || has_secure_computing(next)) {
    + /* slow path here */
    + if (has_secure_computing(prev) &&
    + !has_secure_computing(next)) {
    + write_cr4(read_cr4() & ~X86_CR4_TSD);
    + } else if (!has_secure_computing(prev) &&
    + has_secure_computing(next))
    + write_cr4((read_cr4() | X86_CR4_TSD) & ~X86_CR4_PCE);
    + }
    +#endif
    +}
    +
    +/*
    * This special macro can be used to load a debugging register
    */
    #define loaddebug(thread,r) set_debugreg(thread->debugreg ## r, r)
    @@ -617,6 +646,8 @@ __switch_to(struct task_struct *prev_p,
    memset(tss->io_bitmap, 0xff, prev->io_bitmap_max);
    }
    }
    +
    + disable_tsc(prev_p, next_p);

    return prev_p;
    }
    -
    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: 2006-07-08 11:25    [W:4.286 / U:0.536 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site