All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Rosenberg <dan.j.rosenberg@gmail.com>
To: George Spelvin <linux@horizon.com>
Cc: rmallon@gmail.com, akpm@linux-foundation.org,
	eldad@fogrefinery.com, jkosina@suse.cz,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] printk: Check real user/group id for %pK
Date: Sun, 29 Sep 2013 20:41:05 -0400	[thread overview]
Message-ID: <5248C8A1.3090302@gmail.com> (raw)
In-Reply-To: <20130929234146.31004.qmail@science.horizon.com>

On 09/29/2013 07:41 PM, George Spelvin wrote:
>> Right, so the pppd application is actually doing the correct thing.
> And a CAP_SYSLOG setuid binary that *doesn't* DTRT seems like a more
> immediate security hole than leaking kernel addresses.  After all
> kptr_restrict is optional precisely because the benefit is marginal.
>
> The interesting question is what credentials make sense for %pK outside
> of a seq_printf().  Does it even make sense in a generic printk?  In that
> case, it's the permission of the syslogd that matters rather than the
> process generating the message.
>
>> Will wait and see what others have to say.
> Me, too.  Dan in particular.

Firstly, I wouldn't recommend applying %pK's to printk usage. Removing
all addresses from the kernel syslog compromises its usefulness in
debugging basically anything at all. Additionally, many printk calls are
performed from a context where a capability check would yield
unpredictable (or at least meaningless) results. If you want to restrict
access to the kernel syslog by unprivileged users, that should be done
by enabling CONFIG_DMESG_RESTRICT, which was written for this purpose.

With that out of the way, I don't have a strong opinion on how to handle
this case. The proposed patch solves the problem but may break setuid
applications that expect to be able to read /proc/kallsyms contents
based on euid (and implicitly, capabilities) alone. But then again,
these mythical setuid applications are probably broken in some
situations anyway, because what happens if /proc/kallsyms is set to "2"
(unconditionally replace addresses with 0's)? I also can't think of a
better solution.

-Dan

  reply	other threads:[~2013-09-30  0:41 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-29 22:35 [PATCH] printk: Check real user/group id for %pK Ryan Mallon
2013-09-29 23:15 ` George Spelvin
2013-09-29 23:26   ` Ryan Mallon
2013-09-29 23:41     ` George Spelvin
2013-09-30  0:41       ` Dan Rosenberg [this message]
2013-09-30  0:56         ` Ryan Mallon
2013-09-30  0:59           ` Dan Rosenberg

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=5248C8A1.3090302@gmail.com \
    --to=dan.j.rosenberg@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=eldad@fogrefinery.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@horizon.com \
    --cc=rmallon@gmail.com \
    /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.