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.256 / U:0.828 seconds]
©2003-2014 Jasper Spaans. Advertise on this site