lkml.org 
[lkml]   [2010]   [Nov]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    SubjectRe: [PATCH] Fix dmesg_restrict build failure with CONFIG_EMBEDDED=y and CONFIG_PRINTK=n
    From
    Date
    On Sat, 2010-11-13 at 09:50 -0800, Linus Torvalds wrote:
    > On Sat, Nov 13, 2010 at 9:26 AM, Joe Perches <joe@perches.com> wrote:
    > > dmesg_restrict is guarded by #ifdef CONFIG_PRINTK in kernel.h
    > > Its uses need to be guarded as well.
    > Fair enough, but I think this part:
    > > diff --git a/security/commoncap.c b/security/commoncap.c
    > > index 04b80f9..29f2368 100644
    > > --- a/security/commoncap.c
    > > +++ b/security/commoncap.c
    > > @@ -895,8 +895,10 @@ int cap_syslog(int type, bool from_file)
    > > {
    > > if (type != SYSLOG_ACTION_OPEN && from_file)
    > > return 0;
    > > +#ifdef CONFIG_PRINTK
    > > if (dmesg_restrict && !capable(CAP_SYS_ADMIN))
    > > return -EPERM;
    > > +#endif
    > > if ((type != SYSLOG_ACTION_READ_ALL &&
    > > type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN))
    > > return -EPERM;
    >
    > is incredibly ugly. If CONFIG_PRINTK isn't set, the whole function
    > just becomes pointless, so why guard just that one part of it?
    >
    > So I would suggest guarding the whole thing, and just returning 0 if
    > CONFIG_PRINTK isn't set. Or preferably just move the dmesg_restrict
    > test into do_syslog, and stop playing stupid games with
    > "security_syslog()", which actually goes away if you disable the you
    > disable CONFIG_SECURITY.
    >
    > SECURITY_DMESG_RESTRICT is totally independent of CONFIG_SECURITY, so
    > doing it in security_syslog() was a bug to begin with.
    >
    > Or we should make SECURITY_DMESG_RESTRICT _depend_ on CONFIG_SECURITY,
    > and move it entirely into security/commoncap.c, and not pollute
    > kernel/printk.c at all with it.
    >
    > Anyway, suggested replacement patch attached. Comments?
    >
    > Linus

    Maybe something like this?

    Make CONFIG_SECURITY_DMESG_RESTRICT depend on CONFIG_SECURITY
    Remove dependency on CONFIG_PRINTK

    Uncompiled/untested

    ---
    include/linux/kernel.h | 5 ++++-
    kernel/sysctl.c | 2 ++
    security/Kconfig | 1 +
    security/commoncap.c | 2 ++
    4 files changed, 9 insertions(+), 1 deletions(-)

    diff --git a/include/linux/kernel.h b/include/linux/kernel.h
    index fc3da9e..b9595e8 100644
    --- a/include/linux/kernel.h
    +++ b/include/linux/kernel.h
    @@ -130,6 +130,10 @@ extern const char linux_proc_banner[];

    extern int console_printk[];

    +#ifdef CONFIG_SECURITY_DMESG_RESTRICT
    +extern int dmesg_restrict;
    +#endif
    +
    #define console_loglevel (console_printk[0])
    #define default_message_loglevel (console_printk[1])
    #define minimum_console_loglevel (console_printk[2])
    @@ -293,7 +297,6 @@ extern bool printk_timed_ratelimit(unsigned long *caller_jiffies,
    unsigned int interval_msec);

    extern int printk_delay_msec;
    -extern int dmesg_restrict;

    /*
    * Print a one-time message (analogous to WARN_ONCE() et al):
    diff --git a/kernel/sysctl.c b/kernel/sysctl.c
    index b65bf63..5d7eaab 100644
    --- a/kernel/sysctl.c
    +++ b/kernel/sysctl.c
    @@ -703,6 +703,7 @@ static struct ctl_table kern_table[] = {
    .extra2 = &ten_thousand,
    },
    #endif
    +#ifdef CONFIG_SECURITY_DMESG_RESTRICT
    {
    .procname = "dmesg_restrict",
    .data = &dmesg_restrict,
    @@ -712,6 +713,7 @@ static struct ctl_table kern_table[] = {
    .extra1 = &zero,
    .extra2 = &one,
    },
    +#endif
    {
    .procname = "ngroups_max",
    .data = &ngroups_max,
    diff --git a/security/Kconfig b/security/Kconfig
    index e80da95..c6583d6 100644
    --- a/security/Kconfig
    +++ b/security/Kconfig
    @@ -41,6 +41,7 @@ config KEYS_DEBUG_PROC_KEYS

    config SECURITY_DMESG_RESTRICT
    bool "Restrict unprivileged access to the kernel syslog"
    + depends on SECURITY
    default n
    help
    This enforces restrictions on unprivileged users reading the kernel
    diff --git a/security/commoncap.c b/security/commoncap.c
    index 04b80f9..37759b2 100644
    --- a/security/commoncap.c
    +++ b/security/commoncap.c
    @@ -895,8 +895,10 @@ int cap_syslog(int type, bool from_file)
    {
    if (type != SYSLOG_ACTION_OPEN && from_file)
    return 0;
    +#ifdef CONFIG_SECURITY_DMESG_RESTRICT
    if (dmesg_restrict && !capable(CAP_SYS_ADMIN))
    return -EPERM;
    +#endif
    if ((type != SYSLOG_ACTION_READ_ALL &&
    type != SYSLOG_ACTION_SIZE_BUFFER) && !capable(CAP_SYS_ADMIN))
    return -EPERM;



    \
     
     \ /
      Last update: 2010-11-13 20:55    [W:0.033 / U:0.068 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site