Re: /proc/<pid>/status: incorrect format breaks userland tool

From: Konrad Rzeszutek Wilk
Date: Tue Jan 22 2019 - 00:31:25 EST


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..)