stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Lei Chen <chenl.lei@gmail.com>,
	linux-kernel@vger.kernel.org, stable@vger.kernel.org,
	gregkh@linuxfoundation.org
Cc: tglx@linutronix.de
Subject: Re: /proc/<pid>/status: incorrect format breaks userland tool
Date: Tue, 22 Jan 2019 00:31:08 -0500	[thread overview]
Message-ID: <20190122053108.GA14185@char.us.oracle.com> (raw)
In-Reply-To: <CAB5AJuWz2NgwixCDh3hr4uv8qECfE2m0q49E_5GfecTnGPuXuQ@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4743 bytes --]

On Tue, Jan 22, 2019 at 11:29:16AM +0800, Lei Chen wrote:
> Hi Konrad,

Hi,

CC-ing stable,Greg,and LKML. Pls see attached and inline patch and explanation
at bottom.

> I'm running kernel 4.4.153. When running iotop, I got such failure:
>  # iotop -P
> Traceback (most recent call last):
>   File "/sbin/iotop", line 17, in <module>
>     main()
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 620, in main
>     main_loop()
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 610, in <lambda>
>     main_loop = lambda: run_iotop(options)
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 508, in
> run_iotop
>     return curses.wrapper(run_iotop_window, options)
>   File "/usr/lib/python2.7/curses/wrapper.py", line 43, in wrapper
>     return func(stdscr, *args, **kwds)
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 501, in
> run_iotop_window
>     ui.run()
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 155, in run
>     self.process_list.duration)
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 434, in
> refresh_display
>     lines = self.get_data()
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 415, in get_data
>     return list(map(format, processes))
>   File "/usr/lib/python2.7/site-packages/iotop/ui.py", line 388, in format
>     cmdline = p.get_cmdline()
>   File "/usr/lib/python2.7/site-packages/iotop/data.py", line 292, in
> get_cmdline
>     proc_status = parse_proc_pid_status(self.pid)
>   File "/usr/lib/python2.7/site-packages/iotop/data.py", line 196, in
> parse_proc_pid_status
>     key, value = line.split(':\t', 1)
> ValueError: need more than 1 value to unpack
> 
> Having a little further debug, I found this error is caused by the
> unexpected blank line in /proc/<pid>/status file, like below:
> 
> CapBnd: 0000003fffffffff
> CapAmb: 0000000000000000
> 
> Speculation_Store_Bypass:       vulnerable
> Cpus_allowed:   ff
> 
> Checking the git history, I see you touched the line "seq_printf(m,
> "\nSpeculation_Store_Bypass:\t");". Do you think this additional blank line
> is caused by the leading "\n" of "Speculation_Store_Bypass"?

That is correct.
It looks that the backport missed the change. The v4.4 has:

static inline void task_seccomp(struct seq_file *m, struct task_struct *p)         
{                                                                                  
#ifdef CONFIG_SECCOMP                                                              
        seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);                          
#endif                                                                             
        seq_printf(m, "\nSpeculation_Store_Bypass:\t");      

Upstream has:

tatic inline void task_seccomp(struct seq_file *m, struct task_struct *p)         
{                                                                                  
        seq_put_decimal_ull(m, "NoNewPrivs:\t", task_no_new_privs(p));             
#ifdef CONFIG_SECCOMP                                                              
        seq_put_decimal_ull(m, "\nSeccomp:\t", p->seccomp.mode);                   
#endif                                                                             
        seq_printf(m, "\nSpeculation_Store_Bypass:\t");                         

The af884cd4a5ae6 is the one that removed the '\n' from the end and put it in the
front of 'Seccomp '.

Greg, I am not sure how one would fix this in a stable tree. But the fix is simple
(hadn't tested it..)

From 9e1909f29e1162f2fba190dbab88d1bbcaf0365d Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Tue, 22 Jan 2019 00:27:55 -0500
Subject: [PATCH] Fix: proc: Use underscores for SSBD in 'status'

Upstream af884cd4a5ae6 (not backported) added a '\n' in front
of 'Seccomp' but we have the old format with '\n' at the end.
This causes mayhem with 'Speculation_Store_Bypass' adding an extra
newline breaking tools.

Reported-by:Lei Chen <chenl.lei@gmail.com>
CC: stable@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 fs/proc/array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index cb71cbae606d..60cbaa821164 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -333,7 +333,7 @@ static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
 #ifdef CONFIG_SECCOMP
 	seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
 #endif
-	seq_printf(m, "\nSpeculation_Store_Bypass:\t");
+	seq_printf(m, "Speculation_Store_Bypass:\t");
 	switch (arch_prctl_spec_ctrl_get(p, PR_SPEC_STORE_BYPASS)) {
 	case -EINVAL:
 		seq_printf(m, "unknown");
-- 
2.13.4


> 
> Thanks,
> Lei Chen

[-- Attachment #2: 0001-Fix-proc-Use-underscores-for-SSBD-in-status.patch --]
[-- Type: text/plain, Size: 1179 bytes --]

From 9e1909f29e1162f2fba190dbab88d1bbcaf0365d Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Tue, 22 Jan 2019 00:27:55 -0500
Subject: [PATCH] Fix: proc: Use underscores for SSBD in 'status'

Upstream af884cd4a5ae6 (not backported) added a '\n' in front
of 'Seccomp' but we have the old format with '\n' at the end.
This causes mayhem with 'Speculation_Store_Bypass' adding an extra
newline breaking tools.

Reported-by:Lei Chen <chenl.lei@gmail.com>
CC: stable@vger.kernel.org
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 fs/proc/array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index cb71cbae606d..60cbaa821164 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -333,7 +333,7 @@ static inline void task_seccomp(struct seq_file *m, struct task_struct *p)
 #ifdef CONFIG_SECCOMP
 	seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
 #endif
-	seq_printf(m, "\nSpeculation_Store_Bypass:\t");
+	seq_printf(m, "Speculation_Store_Bypass:\t");
 	switch (arch_prctl_spec_ctrl_get(p, PR_SPEC_STORE_BYPASS)) {
 	case -EINVAL:
 		seq_printf(m, "unknown");
-- 
2.13.4


       reply	other threads:[~2019-01-22  5:31 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAB5AJuWz2NgwixCDh3hr4uv8qECfE2m0q49E_5GfecTnGPuXuQ@mail.gmail.com>
2019-01-22  5:31 ` Konrad Rzeszutek Wilk [this message]
2019-01-22  7:10   ` /proc/<pid>/status: incorrect format breaks userland tool Greg KH

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=20190122053108.GA14185@char.us.oracle.com \
    --to=konrad.wilk@oracle.com \
    --cc=chenl.lei@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /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 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).