Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Signed-off-by: Gwendal Grignou gwendal@chromium.org --- 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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");
On Fri, Jan 11, 2019 at 04:32:12PM -0800, Gwendal Grignou wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Signed-off-by: Gwendal Grignou gwendal@chromium.org
fs/proc/array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Why send this to me? Always use scripts/get_maintainer.pl on a patch to determine who and what lists to send patches to.
And is this a new bug? What commit caused this?
thanks,
greg k-h
On Sat, Jan 12, 2019 at 12:01 AM Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jan 11, 2019 at 04:32:12PM -0800, Gwendal Grignou wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Signed-off-by: Gwendal Grignou gwendal@chromium.org
fs/proc/array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Why send this to me? Always use scripts/get_maintainer.pl on a patch to determine who and what lists to send patches to.
I did, your email address is on the first line: ./scripts/get_maintainer.pl 0001-CHROMIUM-FIXUP-proc-Provide-details-on-speculation-f.patch Greg Kroah-Hartman gregkh@linuxfoundation.org (commit_signer:6/4=100%) "Srivatsa S. Bhat" srivatsa@csail.mit.edu (commit_signer:3/4=75%) Thomas Gleixner tglx@linutronix.de (commit_signer:3/4=75%,authored:1/4=25%,added_lines:3/28=11%) David Woodhouse dwmw@amazon.co.uk (commit_signer:3/4=75%) Bo Gan ganb@vmware.com (commit_signer:3/4=75%) Kees Cook keescook@chromium.org (authored:1/4=25%,added_lines:23/28=82%) Konrad Rzeszutek Wilk konrad.wilk@oracle.com (authored:1/4=25%,removed_lines:1/2=50%) Gwendal Grignou gwendal@chromium.org (authored:1/4=25%,removed_lines:1/2=50%) linux-kernel@vger.kernel.org (open list)
And is this a new bug? What commit caused this?
It is only in 4.4 stable. It has been introduced by: "484964fa3e5a0 proc: Provide details on speculation flaw mitigations"
That patch adds an extra \n in front of "Speculation Store Bypass" that breaks iotop processing of /proc/../status.
Regards,
Gwendal.
thanks,
greg k-h
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org --- v2: Format commit message properly with proper subject and fixes keyword.
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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.20.1.97.g81188d93c3-goog
On Mon, Jan 14, 2019 at 10:21:45AM -0800, Guenter Roeck wrote:
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");
Why isn't this issue showing up in all kernel releases, as this line is still the same in 5.0-rc2?
What makes the 4.4.y and 4.9.y trees so special here?
confused,
greg k-h
On Mon, Jan 14, 2019 at 10:50 AM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jan 14, 2019 at 10:21:45AM -0800, Guenter Roeck wrote:
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");Why isn't this issue showing up in all kernel releases, as this line is still the same in 5.0-rc2?
What makes the 4.4.y and 4.9.y trees so special here?
v4.14 and later:
{ 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");
--- v4.9:
{ #ifdef CONFIG_SECCOMP seq_put_decimal_ull(m, "Seccomp:\t", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> extra newline if CONFIG_SECCOMP=n
--- v4.4:
{ #ifdef CONFIG_SECCOMP seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> always extra newline
Guenter
On Mon, Jan 14, 2019 at 11:01 AM Guenter Roeck groeck@google.com wrote:
On Mon, Jan 14, 2019 at 10:50 AM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jan 14, 2019 at 10:21:45AM -0800, Guenter Roeck wrote:
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");Why isn't this issue showing up in all kernel releases, as this line is still the same in 5.0-rc2?
What makes the 4.4.y and 4.9.y trees so special here?
v4.14 and later:
{ 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");
v4.9:
{ #ifdef CONFIG_SECCOMP seq_put_decimal_ull(m, "Seccomp:\t", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> extra newline if CONFIG_SECCOMP=n
v4.4:
{ #ifdef CONFIG_SECCOMP seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> always extra newline
Guenter
Yeah, this grew out of odd placement of the trailing "\n". I agree it needs fixing universally.
On Mon, Jan 14, 2019 at 11:05 AM Kees Cook keescook@chromium.org wrote:
On Mon, Jan 14, 2019 at 11:01 AM Guenter Roeck groeck@google.com wrote:
On Mon, Jan 14, 2019 at 10:50 AM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jan 14, 2019 at 10:21:45AM -0800, Guenter Roeck wrote:
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");Why isn't this issue showing up in all kernel releases, as this line is still the same in 5.0-rc2?
What makes the 4.4.y and 4.9.y trees so special here?
v4.14 and later:
{ 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");
v4.9:
{ #ifdef CONFIG_SECCOMP seq_put_decimal_ull(m, "Seccomp:\t", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> extra newline if CONFIG_SECCOMP=n
v4.4:
{ #ifdef CONFIG_SECCOMP seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> always extra newline
Guenter
Yeah, this grew out of odd placement of the trailing "\n". I agree it needs fixing universally.
I think we need some guidance on how to fix this problem in 4.4.y and 4.9.y. Backport more of the context patches or stable-release-only patches, possibly with more context explaining the reason ?
Guenter
On Wed, Jan 16, 2019 at 8:06 AM Guenter Roeck groeck@google.com wrote:
On Mon, Jan 14, 2019 at 11:05 AM Kees Cook keescook@chromium.org wrote:
On Mon, Jan 14, 2019 at 11:01 AM Guenter Roeck groeck@google.com wrote:
On Mon, Jan 14, 2019 at 10:50 AM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jan 14, 2019 at 10:21:45AM -0800, Guenter Roeck wrote:
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
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 0c142916a8c7d..f11df9ab4256e 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -334,7 +334,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");Why isn't this issue showing up in all kernel releases, as this line is still the same in 5.0-rc2?
What makes the 4.4.y and 4.9.y trees so special here?
v4.14 and later:
{ 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");
v4.9:
{ #ifdef CONFIG_SECCOMP seq_put_decimal_ull(m, "Seccomp:\t", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> extra newline if CONFIG_SECCOMP=n
v4.4:
{ #ifdef CONFIG_SECCOMP seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> always extra newline
Guenter
Yeah, this grew out of odd placement of the trailing "\n". I agree it needs fixing universally.
I think we need some guidance on how to fix this problem in 4.4.y and 4.9.y. Backport more of the context patches or stable-release-only patches, possibly with more context explaining the reason ?
I think we need 4.4 and 4.9 specific patches that fix it up, and a patch to Linus that regularizes the "always have a trailing \n" style (since the mixture of newlines was what causes this problem in the first place).
On Wed, Jan 16, 2019 at 9:00 AM Kees Cook keescook@chromium.org wrote:
On Wed, Jan 16, 2019 at 8:06 AM Guenter Roeck groeck@google.com wrote:
On Mon, Jan 14, 2019 at 11:05 AM Kees Cook keescook@chromium.org wrote:
On Mon, Jan 14, 2019 at 11:01 AM Guenter Roeck groeck@google.com wrote:
On Mon, Jan 14, 2019 at 10:50 AM Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jan 14, 2019 at 10:21:45AM -0800, Guenter Roeck wrote:
On Mon, Jan 14, 2019 at 10:13 AM Gwendal Grignou gwendal@chromium.org wrote: > > Prevent an empty line in /proc/self/status, allow iotop to work. > > iotop does not like empty lines, fails with: > File "/usr/local/lib64/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 > > [reading /proc/self/status] > > Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations") > > Signed-off-by: Gwendal Grignou gwendal@chromium.org > --- > v2: Format commit message properly with proper subject and fixes > keyword. > You might want to mention that this patch only applies to v4.4.y. v4.9.y has a similar problem, but only if CONFIG_SECCOMP=n, and would require a slightly different patch to fix. Other releases are, as far as I can see, not affected.
Guenter
> 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 0c142916a8c7d..f11df9ab4256e 100644 > --- a/fs/proc/array.c > +++ b/fs/proc/array.c > @@ -334,7 +334,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");
Why isn't this issue showing up in all kernel releases, as this line is still the same in 5.0-rc2?
What makes the 4.4.y and 4.9.y trees so special here?
v4.14 and later:
{ 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");
v4.9:
{ #ifdef CONFIG_SECCOMP seq_put_decimal_ull(m, "Seccomp:\t", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> extra newline if CONFIG_SECCOMP=n
v4.4:
{ #ifdef CONFIG_SECCOMP seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode); ^^^ #endif seq_printf(m, "\nSpeculation_Store_Bypass:\t"); ^^^
-> always extra newline
Guenter
Yeah, this grew out of odd placement of the trailing "\n". I agree it needs fixing universally.
I think we need some guidance on how to fix this problem in 4.4.y and 4.9.y. Backport more of the context patches or stable-release-only patches, possibly with more context explaining the reason ?
I think we need 4.4 and 4.9 specific patches that fix it up, and a patch to Linus that regularizes the "always have a trailing \n" style (since the mixture of newlines was what causes this problem in the first place).
Unfortunately, the lack of trailing newlines is triggered by the use of seq_put_{decimal,hex}_{ull,ll,...}(), which do not add a trailing newline, are used all over the place, and pretty much mandate the use of "\n" at the beginning of the next print statement. I don't think there is an easy way to fix that.
Guenter
-- Kees Cook
On Mon, Jan 14, 2019 at 10:13:36AM -0800, Gwendal Grignou wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Fixes: 84964fa3e5a0 ("proc: Provide details on speculation flaw mitigations")
Signed-off-by: Gwendal Grignou gwendal@chromium.org
v2: Format commit message properly with proper subject and fixes keyword.
fs/proc/array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Now applied, thanks.
greg k-h
On Mon, Jan 14, 2019 at 10:00:54AM -0800, Gwendal Grignou wrote:
On Sat, Jan 12, 2019 at 12:01 AM Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jan 11, 2019 at 04:32:12PM -0800, Gwendal Grignou wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Signed-off-by: Gwendal Grignou gwendal@chromium.org
fs/proc/array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Why send this to me? Always use scripts/get_maintainer.pl on a patch to determine who and what lists to send patches to.
I did, your email address is on the first line: ./scripts/get_maintainer.pl 0001-CHROMIUM-FIXUP-proc-Provide-details-on-speculation-f.patch Greg Kroah-Hartman gregkh@linuxfoundation.org (commit_signer:6/4=100%)
Ah, wait, you are making a patch against the stable kernel tree?
We can't go back in time, you need to work against Linus's latest kernel tree, that's why I am showing up in this list. I'm not the upstream developer for this file.
"Srivatsa S. Bhat" srivatsa@csail.mit.edu (commit_signer:3/4=75%) Thomas Gleixner tglx@linutronix.de (commit_signer:3/4=75%,authored:1/4=25%,added_lines:3/28=11%) David Woodhouse dwmw@amazon.co.uk (commit_signer:3/4=75%) Bo Gan ganb@vmware.com (commit_signer:3/4=75%) Kees Cook keescook@chromium.org (authored:1/4=25%,added_lines:23/28=82%) Konrad Rzeszutek Wilk konrad.wilk@oracle.com (authored:1/4=25%,removed_lines:1/2=50%) Gwendal Grignou gwendal@chromium.org (authored:1/4=25%,removed_lines:1/2=50%) linux-kernel@vger.kernel.org (open list)
And is this a new bug? What commit caused this?
It is only in 4.4 stable. It has been introduced by: "484964fa3e5a0 proc: Provide details on speculation flaw mitigations"
That really is commit fae1fa0fc6cca8beee3ab8ed71d54f9a78fa3f64 in Linus's tree, and is in the 4.4, 4.9, 4.14, 4.16, and 4.17 kernel releases.
So it also is included in 5.0-rc1, if this is still an issue there, please submit the patch to the correct developers and then the patch will be backported to the needed stable kernel trees if you add the correct "Fixes:" and "cc: stable@stable.kernel.org" lines to the patch. Documentation/SubmittingPatches will tell you all about how to do this.
That patch adds an extra \n in front of "Speculation Store Bypass" that breaks iotop processing of /proc/../status.
Why isn't iotop broken in the latest kernel release? It seems to work for me here.
thanks,
greg k-h
On Mon, Jan 14, 2019 at 10:49 AM Greg KH gregkh@linuxfoundation.org wrote:
On Mon, Jan 14, 2019 at 10:00:54AM -0800, Gwendal Grignou wrote:
On Sat, Jan 12, 2019 at 12:01 AM Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jan 11, 2019 at 04:32:12PM -0800, Gwendal Grignou wrote:
Prevent an empty line in /proc/self/status, allow iotop to work.
iotop does not like empty lines, fails with: File "/usr/local/lib64/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
[reading /proc/self/status]
Signed-off-by: Gwendal Grignou gwendal@chromium.org
fs/proc/array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Why send this to me? Always use scripts/get_maintainer.pl on a patch to determine who and what lists to send patches to.
I did, your email address is on the first line: ./scripts/get_maintainer.pl 0001-CHROMIUM-FIXUP-proc-Provide-details-on-speculation-f.patch Greg Kroah-Hartman gregkh@linuxfoundation.org (commit_signer:6/4=100%)
Ah, wait, you are making a patch against the stable kernel tree?
We can't go back in time, you need to work against Linus's latest kernel tree, that's why I am showing up in this list. I'm not the upstream developer for this file.
"Srivatsa S. Bhat" srivatsa@csail.mit.edu (commit_signer:3/4=75%) Thomas Gleixner tglx@linutronix.de (commit_signer:3/4=75%,authored:1/4=25%,added_lines:3/28=11%) David Woodhouse dwmw@amazon.co.uk (commit_signer:3/4=75%) Bo Gan ganb@vmware.com (commit_signer:3/4=75%) Kees Cook keescook@chromium.org (authored:1/4=25%,added_lines:23/28=82%) Konrad Rzeszutek Wilk konrad.wilk@oracle.com (authored:1/4=25%,removed_lines:1/2=50%) Gwendal Grignou gwendal@chromium.org (authored:1/4=25%,removed_lines:1/2=50%) linux-kernel@vger.kernel.org (open list)
And is this a new bug? What commit caused this?
It is only in 4.4 stable. It has been introduced by: "484964fa3e5a0 proc: Provide details on speculation flaw mitigations"
That really is commit fae1fa0fc6cca8beee3ab8ed71d54f9a78fa3f64 in Linus's tree, and is in the 4.4, 4.9, 4.14, 4.16, and 4.17 kernel releases.
So it also is included in 5.0-rc1, if this is still an issue there, please submit the patch to the correct developers and then the patch will be backported to the needed stable kernel trees if you add the correct "Fixes:" and "cc: stable@stable.kernel.org" lines to the patch. Documentation/SubmittingPatches will tell you all about how to do this.
That patch adds an extra \n in front of "Speculation Store Bypass" that breaks iotop processing of /proc/../status.
Why isn't iotop broken in the latest kernel release? It seems to work for me here.
iotop is not broken in ToT, only on 4.4 stable: fae1fa0fc6cca8beee3ab8ed71d54f9a78fa3f64 assumes f7a5f132b447c ("proc: faster /proc/*/status") has been applied. That patch breaks
seq_printf(m, "Seccomp:\t%d\n", p->seccomp.mode);
in 2:
seq_puts(m, "Seccomp:\t"); seq_put_decimal_ull(m, 0, p->seccomp.mode);
and
seq_putc(m, '\n');
f7a5f132b447c is not applied to 4.4 stable. Comparing fae1fa0fc6 with 484964fa3e5a0, the latter adds a 'seq_putc(m, '\n');' whereas it was already there when fae1fa0fc6 was applied. 484964fa3e5a0 is adding an empty line, the patch I propose removes it.
Regards,
Gwendal.
thanks,
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Greg Kroah-Hartman
-
Guenter Roeck -
Gwendal Grignou -
Kees Cook