linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephen Brennan <stephen.s.brennan@oracle.com>
To: Kees Cook <keescook@chromium.org>, Thomas Gleixner <tglx@linutronix.de>
Cc: "Josh Poimboeuf" <jpoimboe@redhat.com>,
	"Vito Caputo" <vcaputo@pengaru.com>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Borislav Petkov" <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>, "Jens Axboe" <axboe@kernel.dk>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Peter Zijlstra" <peterz@infradead.org>,
	"Stefan Metzmacher" <metze@samba.org>,
	"Andy Lutomirski" <luto@kernel.org>,
	"Lai Jiangshan" <laijs@linux.alibaba.com>,
	"Christian Brauner" <christian.brauner@ubuntu.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Kenta.Tada@sony.com" <Kenta.Tada@sony.com>,
	"Daniel Bristot de Oliveira" <bristot@redhat.com>,
	"Michael Weiß" <michael.weiss@aisec.fraunhofer.de>,
	"Anand K Mistry" <amistry@google.com>,
	"Alexey Gladkov" <legion@kernel.org>,
	"Michal Hocko" <mhocko@suse.com>, "Helge Deller" <deller@gmx.de>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	"Andrea Righi" <andrea.righi@canonical.com>,
	"Ohhoon Kwon" <ohoono.kwon@samsung.com>,
	"Kalesh Singh" <kaleshsingh@google.com>,
	"YiFei Zhu" <yifeifz2@illinois.edu>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org,
	linux-fsdevel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH] proc: Disable /proc/$pid/wchan
Date: Thu, 30 Sep 2021 11:05:35 -0700	[thread overview]
Message-ID: <c283c978-2563-06b9-4c21-59bedceda9ea@oracle.com> (raw)
In-Reply-To: <20210923233105.4045080-1-keescook@chromium.org>

On 9/23/21 4:31 PM, Kees Cook wrote:
> The /proc/$pid/wchan file has been broken by default on x86_64 for 4
> years now[1]. As this remains a potential leak of either kernel
> addresses (when symbolization fails) or limited observation of kernel
> function progress, just remove the contents for good.
> 
> Unconditionally set the contents to "0" and also mark the wchan
> field in /proc/$pid/stat with 0.

Hi all,

It looks like there's already been pushback on this idea, but I wanted
to add another voice from a frequent user of /proc/$pid/wchan (via PS).
Much of my job involves diagnosing kernel issues and performance issues
on stable kernels, frequently on production systems where I can't do
anything too invasive. wchan is incredibly useful for these situations,
so much so that we store regular snapshots of ps output, and we expand
the size of the WCHAN column to fit more data (e.g. ps -e -o
pid,wchan=WCHAN-WIDE-COLUMN). Disabling wchan would remove a critical
tool for me and my team.

 From my our team's feedback:
1. It's fine if this needs to have CAP_SYS_ADMIN to read for tasks not
    owned by the calling user; and for non-admin, if the symbolization
    fails, to return 0 just like kallsyms does for unprivileged users.
2. We don't care about the stack of an actively running process
    (/proc/$pid/stack is there for that). We only need WCHAN for
    understanding why a task is blocked.
3. Keeping the function / symbol name in the wchan is ideal (so we can
    pinpoint the exact area that a task is blocked at).

> This leaves kernel/sched/fair.c as the only user of get_wchan(). But
> again, since this was broken for 4 years, was this profiling logic
> actually doing anything useful?

This was only broken with CONFIG_UNWINDER_ORC. You may say this is the
default, but Ubuntu's latest kernel (5.11 in Hirsute) still ships with
CONFIG_UNWINDER_FRAME_POINTER, and many other distributions are the
same. Stable distributions have a lag time picking up new code, and even
longer lag picking up new configurations -- even new defaults.
(Especially when frame pointers are so useful for debugging...) So
saying that this was broken for 4 years is at best misleading. Plenty of
users have been happily using recent kernels when this was supposedly
"broken", on valid configurations, without any issues.

It looks like we've backed off of the decision to rip out 
/proc/$pid/wchan, but I just wanted to chime in, since it feels like the 
discussion is happening without much input from users.

Thanks,
Stephen

  parent reply	other threads:[~2021-09-30 18:07 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-23 23:31 Kees Cook
2021-09-23 23:38 ` Kees Cook
2021-09-24 13:31   ` Peter Zijlstra
2021-09-23 23:49 ` Vito Caputo
2021-09-24  0:08   ` Jann Horn
2021-09-24  0:22     ` Vito Caputo
2021-09-24  1:16       ` Kees Cook
2021-09-24  1:34         ` Vito Caputo
2021-09-24  1:42           ` Kees Cook
2021-09-24 13:54         ` Mark Rutland
2021-09-24 14:26           ` Kees Cook
2021-09-27  9:03             ` Mark Rutland
2021-09-27 18:07               ` Kees Cook
2021-09-27 20:50                 ` Josh Poimboeuf
2021-09-29 18:54                   ` Kees Cook
2021-09-29 19:00                     ` Mark Brown
2021-09-29 19:26                       ` Kees Cook
2021-09-29 19:40                     ` Peter Zijlstra
2021-09-29 21:10                       ` Josh Poimboeuf
2021-09-27  9:16           ` David Laight
2021-09-29  8:48           ` Peter Zijlstra
2021-09-24  2:13 ` Andrew Morton
2021-09-24  6:04   ` Kees Cook
2021-09-24 13:29 ` Peter Zijlstra
2021-09-30 18:05 ` Stephen Brennan [this message]
2021-09-30 18:12   ` Kees Cook

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=c283c978-2563-06b9-4c21-59bedceda9ea@oracle.com \
    --to=stephen.s.brennan@oracle.com \
    --cc=Kenta.Tada@sony.com \
    --cc=akpm@linux-foundation.org \
    --cc=amistry@google.com \
    --cc=andrea.righi@canonical.com \
    --cc=axboe@kernel.dk \
    --cc=bp@alien8.de \
    --cc=bristot@redhat.com \
    --cc=christian.brauner@ubuntu.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=deller@gmx.de \
    --cc=ebiederm@xmission.com \
    --cc=hpa@zytor.com \
    --cc=jpoimboe@redhat.com \
    --cc=kaleshsingh@google.com \
    --cc=keescook@chromium.org \
    --cc=laijs@linux.alibaba.com \
    --cc=legion@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=metze@samba.org \
    --cc=mhocko@suse.com \
    --cc=michael.weiss@aisec.fraunhofer.de \
    --cc=mingo@redhat.com \
    --cc=ohoono.kwon@samsung.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=vcaputo@pengaru.com \
    --cc=x86@kernel.org \
    --cc=yifeifz2@illinois.edu \
    --subject='Re: [PATCH] proc: Disable /proc/$pid/wchan' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).