linux-kernel - Re: [PATCH] Fix dmesg_restrict build failure with CONFIG_EMBEDDED=y and CONFIG

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTi=ut2O+mCbs1jOyg-aP=E+48h1B-MH8NxK5XyVf@mail.gmail.com>
Date:	Sat, 13 Nov 2010 09:50:22 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Joe Perches <joe@...ches.com>
Cc:	Dan Rosenberg <drosenberg@...curity.com>,
	LKML <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...e.hu>,
	Eugene Teo <eugeneteo@...nel.org>,
	Kees Cook <kees.cook@...onical.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] Fix dmesg_restrict build failure with CONFIG_EMBEDDED=y
 and CONFIG_PRINTK=n

On Sat, Nov 13, 2010 at 9:26 AM, Joe Perches <joe@...ches.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

View attachment "patch.diff" of type "text/x-patch" (1475 bytes)