All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>,
	Gergely Nagy <algernon@balabit.hu>,
	david@lang.hm, Alan Cox <alan@lxorguk.ukuu.org.uk>,
	Marc Koschewski <marc@osknowledge.org>,
	lkml <linux-kernel@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	Nick Bowler <nbowler@elliptictech.com>
Subject: [PATCH v2] cap_syslog: don't use WARN_ONCE for CAP_SYS_ADMIN deprecation warning
Date: Mon, 8 Aug 2011 06:22:43 +0200	[thread overview]
Message-ID: <20110808042243.GG19551@elie.gateway.2wire.net> (raw)
In-Reply-To: <CA+55aFw0d0bGQhtLB602=_i-PAMmkBs0vRocvtRZUU33xS8=4A@mail.gmail.com>

syslog-ng versions before 3.3.0beta1 (2011-05-12) assume that
CAP_SYS_ADMIN is sufficient to access syslog, so ever since CAP_SYSLOG
was introduced (2010-11-25) they have triggered a warning.
v2.6.38-rc5~46 (cap_syslog: accept CAP_SYS_ADMIN for now, 2011-02-10)
improved matters a little by making syslog-ng work again, just keeping
the WARN_ONCE().  But still, this is a warning that writes a stack
trace we don't care about to syslog, sets a taint flag, and alarms
sysadmins when nothing worse has happened than use of an old userspace
with a recent kernel.

Convert the WARN_ONCE to a printk_once to avoid that while continuing
to give userspace developers a hint that this is an unwanted
backward-compatibility feature and won't be around forever.

Reported-by: Ralf Hildebrandt <ralf.hildebrandt@charite.de>
Reported-by: Niels <zorglub_olsen@hotmail.com>
Reported-by: Paweł Sikora <pluto@agmk.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Liked-by: Gergely Nagy <algernon@madhouse-project.org>
Acked-by: Serge Hallyn <serge@hallyn.com>
Acked-by: James Morris <jmorris@namei.org>
---
Linus Torvalds wrote:

> The problem with this one is that converting away from a warning
> doesn't just get rid of all the ugliness, it gets rid of some of the
> *useful* information in the warning too.
>
> In particular, when you have a warning like "Attempting to access
> syslog", you'd better tell people *who* is attempting to access
> syslog, so that they can fix it or complain to the right person.

True.  I can't imagine people wanting to know much more than the
process's command line and pid, which is basically what the WARN_ONCE
gives (though it also gives registers).  Here's an updated patch.

I've carried over the acks from last time; hopefully that's okay.
Thanks for your help.

 kernel/printk.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/printk.c b/kernel/printk.c
index 37dff342..836a2ae0 100644
--- a/kernel/printk.c
+++ b/kernel/printk.c
@@ -318,8 +318,10 @@ static int check_syslog_permissions(int type, bool from_file)
 			return 0;
 		/* For historical reasons, accept CAP_SYS_ADMIN too, with a warning */
 		if (capable(CAP_SYS_ADMIN)) {
-			WARN_ONCE(1, "Attempt to access syslog with CAP_SYS_ADMIN "
-				 "but no CAP_SYSLOG (deprecated).\n");
+			printk_once(KERN_WARNING "%s (%d): "
+				 "Attempt to access syslog with CAP_SYS_ADMIN "
+				 "but no CAP_SYSLOG (deprecated).\n",
+				 current->comm, task_pid_nr(current));
 			return 0;
 		}
 		return -EPERM;
-- 
1.7.6


      reply	other threads:[~2011-08-08  4:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-10 14:40 [PATCH 1/1] cap_syslog: don't refuse cap_sys_admin for now (v3) Serge E. Hallyn
2011-02-10 19:16 ` Linus Torvalds
2011-02-10 22:43   ` Serge E. Hallyn
2011-02-10 22:59   ` James Morris
2011-02-11 16:32   ` Serge E. Hallyn
2011-08-03 16:48   ` [PATCH/RFC] cap_syslog: make CAP_SYS_ADMIN deprecation notice less alarming Jonathan Nieder
2011-08-04  1:28     ` James Morris
2011-08-04  4:39     ` Serge E. Hallyn
2011-08-05 13:45     ` James Morris
2011-08-05 18:50     ` Linus Torvalds
2011-08-08  4:22       ` Jonathan Nieder [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110808042243.GG19551@elie.gateway.2wire.net \
    --to=jrnieder@gmail.com \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=algernon@balabit.hu \
    --cc=david@lang.hm \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc@osknowledge.org \
    --cc=nbowler@elliptictech.com \
    --cc=serge@hallyn.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.