[PATCH 0/3] Dump command line of faulting process to syslog

[PATCH 0/3] Dump command line of faulting process to syslog

[Helge Deller]

This patch series allows the arch-specific kernel fault handlers to dump
in addition to the typical info (IP address, fault type, backtrace and so on)
the command line of the faulting process.

The motivation for this patch is that it's sometimes quite hard to find out and
annoying to not know which program *exactly* faulted when looking at the syslog.

Some examples from the syslog are:

On parisc:
   do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]
   CPU: 1 PID: 13472 Comm: cc1 Tainted: G            E     5.10.133+ #45
   Hardware name: 9000/785/C8000

-> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.

With this patch series, the kernel now prints in addition:
   cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu -D USE_MINIINTERPRETER -D NO_REGS -D _HPUX_SOURCE -D NOSMP -D THREADED_RTS -include /build/ghc/ghc-9.0.2/includes/dist-install/build/ghcversion.h -iquote compiler/GHC/Iface -quiet -dumpdir /tmp/ghc13413_0/ -dumpbase ghc_5.hc -dumpbase-ext .hc -O -Wimplicit -fno-PIC -fwrapv -fno-builtin -fno-strict-aliasing -o /tmp/ghc13413_0/ghc_5.s

-> now we know that cc1 crashed while compiling some haskell code.

Another parisc example:
   do_page_fault() command='ld.so.1' type=15 address=0x565921d8 in libc.so[f7339000+1bb000]
   CPU: 1 PID: 1151 Comm: cc1 Tainted: G            E     5.10.133+ #45
   Hardware name: 9000/785/C8000

-> apparently here a program from the glibc testsuite segfaulted.

With this patch we now additionally get:
   ld.so.1[1151] cmdline: /home/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/gnu/glibc/objdir:/home/gnu/glibc/objdir/math:/home/gnu/
        /home/gnu/glibc/objdir/malloc/tst-safe-linking-malloc-hugetlb1

-> it was the tst-safe-linking-malloc-hugetlb1 testcase which faulted.

An example of a typical x86 fault shows up as:
   crash[2326]: segfault at 0 ip 0000561a7969c12e sp 00007ffe97a05630 error 6 in crash[561a7969c000+1000]
   Code: 68 ff ff ff c6 05 19 2f 00 00 01 5d c3 0f 1f 80 00 00 00 00 c3 0f 1f 80 00 00 00 00 e9 7b ff ff ff 55 48 89 e5 b8 00 00 00 00 <c7> 00 01 00 00 00 b8 00 00 00 00 5d c3 0f 1f 44 00 00 41 57 4c 8d

-> with this patch we now will see the whole command line:
   crash[2326] cmdline: ./crash test_write_to_page_0

The patches are relatively small, and reuses functions which are used
to create the output for the /proc/<pid>/cmdline files.

This is the version 1 of the patch series.
I'm interested if people find this useful too, and if so, I'm
happy for any feedback on those patches.

Thanks!
Helge

Helge Deller (3):
  proc: Add get_task_cmdline_kernel() function
  lib/dump_stack: Add dump_stack_print_cmdline() and wire up in
    dump_stack_print_info()
  x86/fault: Dump command line of faulting process to syslog

 arch/x86/mm/fault.c     |  2 ++
 fs/proc/base.c          | 68 +++++++++++++++++++++++++++--------------
 include/linux/printk.h  |  5 +++
 include/linux/proc_fs.h |  5 +++
 lib/dump_stack.c        | 17 +++++++++++
 5 files changed, 74 insertions(+), 23 deletions(-)

--
2.37.1

[PATCH 1/3] proc: Add get_task_cmdline_kernel() function

[Helge Deller]

Add a new function get_task_cmdline_kernel() which reads the command
line of a process into a kernel buffer. This command line can then be
dumped by arch code to print additional debug info on how a faulting
process was started.

The new function re-uses the existing code which provides the cmdline
for the procfs. For that the existing functions were modified so that
the buffer page is allocated outside of get_mm_proctitle() and
get_mm_cmdline() and instead provided as parameter.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 fs/proc/base.c          | 68 +++++++++++++++++++++++++++--------------
 include/linux/proc_fs.h |  5 +++
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 8dfa36a99c74..4da9a8b3c7d1 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -217,20 +217,17 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
  */
 static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
 				size_t count, unsigned long pos,
-				unsigned long arg_start)
+				unsigned long arg_start, char *page)
 {
-	char *page;
 	int ret, got;
+	size_t size;

-	if (pos >= PAGE_SIZE)
+	size = min_t(size_t, PAGE_SIZE, count);
+	if (pos >= size)
 		return 0;

-	page = (char *)__get_free_page(GFP_KERNEL);
-	if (!page)
-		return -ENOMEM;
-
 	ret = 0;
-	got = access_remote_vm(mm, arg_start, page, PAGE_SIZE, FOLL_ANON);
+	got = access_remote_vm(mm, arg_start, page, size, FOLL_ANON);
 	if (got > 0) {
 		int len = strnlen(page, got);

@@ -238,7 +235,9 @@ static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
 		if (len < got)
 			len++;

-		if (len > pos) {
+		if (!buf)
+			ret = len;
+		else if (len > pos) {
 			len -= pos;
 			if (len > count)
 				len = count;
@@ -248,16 +247,15 @@ static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
 			ret = len;
 		}
 	}
-	free_page((unsigned long)page);
 	return ret;
 }

 static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
-			      size_t count, loff_t *ppos)
+			      size_t count, loff_t *ppos, char *page)
 {
 	unsigned long arg_start, arg_end, env_start, env_end;
 	unsigned long pos, len;
-	char *page, c;
+	char c;

 	/* Check if process spawned far enough to have cmdline. */
 	if (!mm->env_end)
@@ -283,7 +281,7 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 	len = env_end - arg_start;

 	/* We're not going to care if "*ppos" has high bits set */
-	pos = *ppos;
+	pos = ppos ? *ppos : 0;
 	if (pos >= len)
 		return 0;
 	if (count > len - pos)
@@ -299,7 +297,7 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 	 * pos is 0, and set a flag in the 'struct file'.
 	 */
 	if (access_remote_vm(mm, arg_end-1, &c, 1, FOLL_ANON) == 1 && c)
-		return get_mm_proctitle(mm, buf, count, pos, arg_start);
+		return get_mm_proctitle(mm, buf, count, pos, arg_start, page);

 	/*
 	 * For the non-setproctitle() case we limit things strictly
@@ -311,10 +309,6 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 	if (count > arg_end - pos)
 		count = arg_end - pos;

-	page = (char *)__get_free_page(GFP_KERNEL);
-	if (!page)
-		return -ENOMEM;
-
 	len = 0;
 	while (count) {
 		int got;
@@ -323,7 +317,8 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 		got = access_remote_vm(mm, pos, page, size, FOLL_ANON);
 		if (got <= 0)
 			break;
-		got -= copy_to_user(buf, page, got);
+		if (buf)
+			got -= copy_to_user(buf, page, got);
 		if (unlikely(!got)) {
 			if (!len)
 				len = -EFAULT;
@@ -335,12 +330,11 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 		count -= got;
 	}

-	free_page((unsigned long)page);
 	return len;
 }

 static ssize_t get_task_cmdline(struct task_struct *tsk, char __user *buf,
-				size_t count, loff_t *pos)
+				size_t count, loff_t *pos, char *page)
 {
 	struct mm_struct *mm;
 	ssize_t ret;
@@ -349,23 +343,51 @@ static ssize_t get_task_cmdline(struct task_struct *tsk, char __user *buf,
 	if (!mm)
 		return 0;

-	ret = get_mm_cmdline(mm, buf, count, pos);
+	ret = get_mm_cmdline(mm, buf, count, pos, page);
 	mmput(mm);
 	return ret;
 }

+/*
+ * Place up to maxcount chars of the command line of the process into the
+ * cmdline buffer.
+ */
+void get_task_cmdline_kernel(struct task_struct *tsk,
+			char *cmdline, size_t maxcount)
+{
+	int i;
+
+	memset(cmdline, 0, maxcount);
+	get_task_cmdline(tsk, NULL, maxcount - 1, NULL, cmdline);
+
+	/* remove NULs between parameters */
+	for (i = 0; i < maxcount - 2; i++) {
+		if (cmdline[i])
+			continue;
+		if (cmdline[i+1] == 0)
+			break;
+		cmdline[i] = ' ';
+	}
+}
+
 static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,
 				     size_t count, loff_t *pos)
 {
 	struct task_struct *tsk;
 	ssize_t ret;
+	char *page;

 	BUG_ON(*pos < 0);

 	tsk = get_proc_task(file_inode(file));
 	if (!tsk)
 		return -ESRCH;
-	ret = get_task_cmdline(tsk, buf, count, pos);
+	page = (char *)__get_free_page(GFP_KERNEL);
+	if (page) {
+		ret = get_task_cmdline(tsk, buf, count, pos, page);
+		free_page((unsigned long)page);
+	} else
+		ret = -ENOMEM;
 	put_task_struct(tsk);
 	if (ret > 0)
 		*pos += ret;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 81d6e4ec2294..9a256e86205c 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -158,6 +158,9 @@ int proc_pid_arch_status(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task);
 #endif /* CONFIG_PROC_PID_ARCH_STATUS */

+void get_task_cmdline_kernel(struct task_struct *tsk,
+			char *cmdline, size_t maxcount);
+
 #else /* CONFIG_PROC_FS */

 static inline void proc_root_init(void)
@@ -216,6 +219,8 @@ static inline struct pid *tgid_pidfd_to_pid(const struct file *file)
 	return ERR_PTR(-EBADF);
 }

+static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
+
 #endif /* CONFIG_PROC_FS */

 struct net;
--
2.37.1

[PATCH 2/3] lib/dump_stack: Add dump_stack_print_cmdline() and wire up in dump_stack_print_info()

[Helge Deller]

Add the function dump_stack_print_cmdline() which can be used by arch
code to print the command line of the current processs.  This function
is useful in arch code when dumping information for a faulting process.

Wire this function up in the dump_stack_print_info() function to include
the dumping of the command line for architectures which use
dump_stack_print_info().

As an example, with this patch a failing glibc testcase (which uses
ld.so.1 as starting program) up to now reported just "ld.so.1" failing:

 do_page_fault() command='ld.so.1' type=15 address=0x565921d8 in libc.so[f7339000+1bb000]
 trap #15: Data TLB miss fault, vm_start = 0x0001a000, vm_end = 0x0001b000

and now it reports in addition:

 ld.so.1[1151] cmdline: /home/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/gnu/glibc/objdir:/home/gnu/glibc/objdir/math:/home/gnu/
    /home/gnu/glibc/objdir/malloc/tst-safe-linking-malloc-hugetlb1

Signed-off-by: Helge Deller <deller@gmx.de>
---
 include/linux/printk.h |  5 +++++
 lib/dump_stack.c       | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index cf7d666ab1f8..5290a32a197d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -191,6 +191,7 @@ u32 log_buf_len_get(void);
 void log_buf_vmcoreinfo_setup(void);
 void __init setup_log_buf(int early);
 __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
+void dump_stack_print_cmdline(const char *log_lvl);
 void dump_stack_print_info(const char *log_lvl);
 void show_regs_print_info(const char *log_lvl);
 extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold;
@@ -262,6 +263,10 @@ static inline __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...)
 {
 }

+static inline void dump_stack_print_cmdline(const char *log_lvl)
+{
+}
+
 static inline void dump_stack_print_info(const char *log_lvl)
 {
 }
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 83471e81501a..c9b6a4c0c4e1 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -14,6 +14,7 @@
 #include <linux/kexec.h>
 #include <linux/utsname.h>
 #include <linux/stop_machine.h>
+#include <linux/proc_fs.h>

 static char dump_stack_arch_desc_str[128];

@@ -45,6 +46,20 @@ void __init dump_stack_set_arch_desc(const char *fmt, ...)
 #define BUILD_ID_VAL ""
 #endif

+/**
+ * dump_stack_print_cmdline - print the command line of current process
+ * @log_lvl: log level
+ */
+void dump_stack_print_cmdline(const char *log_lvl)
+{
+	char cmdline[256];
+
+	/* get command line */
+	get_task_cmdline_kernel(current, cmdline, sizeof(cmdline));
+	printk("%s%s[%d] cmdline: %s\n", log_lvl, current->comm,
+		current->pid, cmdline);
+}
+
 /**
  * dump_stack_print_info - print generic debug info for dump_stack()
  * @log_lvl: log level
@@ -62,6 +77,8 @@ void dump_stack_print_info(const char *log_lvl)
 	       (int)strcspn(init_utsname()->version, " "),
 	       init_utsname()->version, BUILD_ID_VAL);

+	dump_stack_print_cmdline(log_lvl);
+
 	if (dump_stack_arch_desc_str[0] != '\0')
 		printk("%sHardware name: %s\n",
 		       log_lvl, dump_stack_arch_desc_str);
--
2.37.1

[PATCH 3/3] x86/fault: Dump command line of faulting process to syslog

[Helge Deller]

If a process segfaults, include the command line of the faulting process
in the syslog.

In the example below, the "crash" program (which simply writes zero to address 0)
was called with the parameters "this is a test":

 crash[2326]: segfault at 0 ip 0000561a7969c12e sp 00007ffe97a05630 error 6 in crash[561a7969c000+1000]
 crash[2326] cmdline: ./crash this is a test
 Code: 68 ff ff ff c6 05 19 2f 00 00 01 5d c3 0f 1f 80 00 00 00 00 c3 0f 1f 80 00 00 00 00 e9 7b ff ff ff 55 48 89 e5 b8 00 00 00 00 <c7> 00 01 00 00 00 b8 00 00 00 00 5d c3 0f 1f 44 00 00 41 57 4c 8d

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/x86/mm/fault.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index fad8faa29d04..d4e21c402e29 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -784,6 +784,8 @@ show_signal_msg(struct pt_regs *regs, unsigned long error_code,

 	printk(KERN_CONT "\n");

+	dump_stack_print_cmdline(loglvl);
+
 	show_opcodes(regs, loglvl);
 }

--
2.37.1

Re: [PATCH 0/3] Dump command line of faulting process to syslog

[Josh Triplett]

On Mon, Aug 01, 2022 at 05:20:13PM +0200, Helge Deller wrote:
> This patch series allows the arch-specific kernel fault handlers to dump
> in addition to the typical info (IP address, fault type, backtrace and so on)
> the command line of the faulting process.
> 
> The motivation for this patch is that it's sometimes quite hard to find out and
> annoying to not know which program *exactly* faulted when looking at the syslog.
> 
> Some examples from the syslog are:
> 
> On parisc:
>    do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]
>    CPU: 1 PID: 13472 Comm: cc1 Tainted: G            E     5.10.133+ #45
>    Hardware name: 9000/785/C8000
> 
> -> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.
> 
> With this patch series, the kernel now prints in addition:
>    cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu -D USE_MINIINTERPRETER -D NO_REGS -D _HPUX_SOURCE -D NOSMP -D THREADED_RTS -include /build/ghc/ghc-9.0.2/includes/dist-install/build/ghcversion.h -iquote compiler/GHC/Iface -quiet -dumpdir /tmp/ghc13413_0/ -dumpbase ghc_5.hc -dumpbase-ext .hc -O -Wimplicit -fno-PIC -fwrapv -fno-builtin -fno-strict-aliasing -o /tmp/ghc13413_0/ghc_5.s
> 
> -> now we know that cc1 crashed while compiling some haskell code.

This does seem really useful for debugging.

However, it's also an information disclosure in various ways. The
arguments of a program are often more sensitive than the name, and logs
have a tendency to end up in various places, such as bug reports.

An example of how this can be an issue:
- You receive an email or other message with a sensitive link to follow
- You open the link, which launches `firefox https://...`
- You continue browsing from that window
- Firefox crashes (and recovers and restarts, so you don't think
  anything of it)
- Later, you report a bug on a different piece of software, and the bug
  reporting process includes a copy of the kernel log

I am *not* saying that we shouldn't do this; it seems quite helpful.
However, I think we need to arrange to treat this as sensitive
information, similar to kptr_restrict. (It would also be helpful if
there was a way to tell `dmesg` "please give me the redacted version of
the log", and bug reporting software used that by default.)

- Josh Triplett

Re: [PATCH 0/3] Dump command line of faulting process to syslog

[Helge Deller]

On 8/1/22 18:57, Josh Triplett wrote:
> On Mon, Aug 01, 2022 at 05:20:13PM +0200, Helge Deller wrote:
>> This patch series allows the arch-specific kernel fault handlers to dump
>> in addition to the typical info (IP address, fault type, backtrace and so on)
>> the command line of the faulting process.
>>
>> The motivation for this patch is that it's sometimes quite hard to find out and
>> annoying to not know which program *exactly* faulted when looking at the syslog.
>>
>> Some examples from the syslog are:
>>
>> On parisc:
>>    do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]
>>    CPU: 1 PID: 13472 Comm: cc1 Tainted: G            E     5.10.133+ #45
>>    Hardware name: 9000/785/C8000
>>
>> -> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.
>>
>> With this patch series, the kernel now prints in addition:
>>    cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu -D USE_MINIINTERPRETER -D NO_REGS -D _HPUX_SOURCE -D NOSMP -D THREADED_RTS -include /build/ghc/ghc-9.0.2/includes/dist-install/build/ghcversion.h -iquote compiler/GHC/Iface -quiet -dumpdir /tmp/ghc13413_0/ -dumpbase ghc_5.hc -dumpbase-ext .hc -O -Wimplicit -fno-PIC -fwrapv -fno-builtin -fno-strict-aliasing -o /tmp/ghc13413_0/ghc_5.s
>>
>> -> now we know that cc1 crashed while compiling some haskell code.
>
> This does seem really useful for debugging.

Yes.

> However, it's also an information disclosure in various ways. The
> arguments of a program are often more sensitive than the name, and logs
> have a tendency to end up in various places, such as bug reports.
>
> An example of how this can be an issue:
> - You receive an email or other message with a sensitive link to follow
> - You open the link, which launches `firefox https://...`
> - You continue browsing from that window
> - Firefox crashes (and recovers and restarts, so you don't think
>   anything of it)
> - Later, you report a bug on a different piece of software, and the bug
>   reporting process includes a copy of the kernel log

Yes, that's a possible way how such information can leak.

> I am *not* saying that we shouldn't do this; it seems quite helpful.
> However, I think we need to arrange to treat this as sensitive
> information, similar to kptr_restrict.

I wonder what the best solution could be.

A somewhat trivial solution is to combine it with the dmesg_restrict sysctl, e.g.:

* When ``dmesg_restrict`` is set to 0 there are no restrictions for users to read
dmesg. In this case my patch would limit the information (based on example above):
    cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 [note: other parameters hidden due to dmesg_restrict=0 sysctl]
So it would show the full argv[0] with a hint that people would need to change dmesg_restrict.

* When ``dmesg_restrict`` is set to 1, users must have ``CAP_SYSLOG`` to use dmesg(8)
and the patch could output all parameters:
     cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu ....

That would of course still leave few possible corner-cases where information
could leak, but since usually programs shouldn't crash and that
people usually shouldn't put sensitive information into the parameter
list directly, it's somewhat unlikely to happen.

Another different solution would be to add another sysctl.

Any other ideas?

Helge

Re: [PATCH 0/3] Dump command line of faulting process to syslog

[Josh Triplett]

On Tue, Aug 02, 2022 at 09:40:50PM +0200, Helge Deller wrote:
> On 8/1/22 18:57, Josh Triplett wrote:
> > On Mon, Aug 01, 2022 at 05:20:13PM +0200, Helge Deller wrote:
> >> This patch series allows the arch-specific kernel fault handlers to dump
> >> in addition to the typical info (IP address, fault type, backtrace and so on)
> >> the command line of the faulting process.
> >>
> >> The motivation for this patch is that it's sometimes quite hard to find out and
> >> annoying to not know which program *exactly* faulted when looking at the syslog.
> >>
> >> Some examples from the syslog are:
> >>
> >> On parisc:
> >>    do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]
> >>    CPU: 1 PID: 13472 Comm: cc1 Tainted: G            E     5.10.133+ #45
> >>    Hardware name: 9000/785/C8000
> >>
> >> -> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.
> >>
> >> With this patch series, the kernel now prints in addition:
> >>    cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu -D USE_MINIINTERPRETER -D NO_REGS -D _HPUX_SOURCE -D NOSMP -D THREADED_RTS -include /build/ghc/ghc-9.0.2/includes/dist-install/build/ghcversion.h -iquote compiler/GHC/Iface -quiet -dumpdir /tmp/ghc13413_0/ -dumpbase ghc_5.hc -dumpbase-ext .hc -O -Wimplicit -fno-PIC -fwrapv -fno-builtin -fno-strict-aliasing -o /tmp/ghc13413_0/ghc_5.s
> >>
> >> -> now we know that cc1 crashed while compiling some haskell code.
> >
> > This does seem really useful for debugging.
> 
> Yes.
> 
> > However, it's also an information disclosure in various ways. The
> > arguments of a program are often more sensitive than the name, and logs
> > have a tendency to end up in various places, such as bug reports.
> >
> > An example of how this can be an issue:
> > - You receive an email or other message with a sensitive link to follow
> > - You open the link, which launches `firefox https://...`
> > - You continue browsing from that window
> > - Firefox crashes (and recovers and restarts, so you don't think
> >   anything of it)
> > - Later, you report a bug on a different piece of software, and the bug
> >   reporting process includes a copy of the kernel log
> 
> Yes, that's a possible way how such information can leak.
> 
> > I am *not* saying that we shouldn't do this; it seems quite helpful.
> > However, I think we need to arrange to treat this as sensitive
> > information, similar to kptr_restrict.
> 
> I wonder what the best solution could be.
> 
> A somewhat trivial solution is to combine it with the dmesg_restrict sysctl, e.g.:
> 
> * When ``dmesg_restrict`` is set to 0 there are no restrictions for users to read
> dmesg. In this case my patch would limit the information (based on example above):
>     cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 [note: other parameters hidden due to dmesg_restrict=0 sysctl]
> So it would show the full argv[0] with a hint that people would need to change dmesg_restrict.
> 
> * When ``dmesg_restrict`` is set to 1, users must have ``CAP_SYSLOG`` to use dmesg(8)
> and the patch could output all parameters:
>      cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu ....
> 
> That would of course still leave few possible corner-cases where information
> could leak, but since usually programs shouldn't crash and that
> people usually shouldn't put sensitive information into the parameter
> list directly, it's somewhat unlikely to happen.
> 
> Another different solution would be to add another sysctl.
> 
> Any other ideas?

I don't think we should overload the meaning of dmesg_restrict. But
overloading kptr_restrict seems reasonable to me. (Including respecting
kptr_restrict==2 by not showing this at all.)

Re: [PATCH 0/3] Dump command line of faulting process to syslog

[Helge Deller]

On 8/4/22 10:09, Josh Triplett wrote:
> On Tue, Aug 02, 2022 at 09:40:50PM +0200, Helge Deller wrote:
>> On 8/1/22 18:57, Josh Triplett wrote:
>>> On Mon, Aug 01, 2022 at 05:20:13PM +0200, Helge Deller wrote:
>>>> This patch series allows the arch-specific kernel fault handlers to dump
>>>> in addition to the typical info (IP address, fault type, backtrace and so on)
>>>> the command line of the faulting process.
>>>>
>>>> The motivation for this patch is that it's sometimes quite hard to find out and
>>>> annoying to not know which program *exactly* faulted when looking at the syslog.
>>>>
>>>> Some examples from the syslog are:
>>>>
>>>> On parisc:
>>>>    do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]
>>>>    CPU: 1 PID: 13472 Comm: cc1 Tainted: G            E     5.10.133+ #45
>>>>    Hardware name: 9000/785/C8000
>>>>
>>>> -> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.
>>>>
>>>> With this patch series, the kernel now prints in addition:
>>>>    cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu -D USE_MINIINTERPRETER -D NO_REGS -D _HPUX_SOURCE -D NOSMP -D THREADED_RTS -include /build/ghc/ghc-9.0.2/includes/dist-install/build/ghcversion.h -iquote compiler/GHC/Iface -quiet -dumpdir /tmp/ghc13413_0/ -dumpbase ghc_5.hc -dumpbase-ext .hc -O -Wimplicit -fno-PIC -fwrapv -fno-builtin -fno-strict-aliasing -o /tmp/ghc13413_0/ghc_5.s
>>>>
>>>> -> now we know that cc1 crashed while compiling some haskell code.
>>>
>>> This does seem really useful for debugging.
>>
>> Yes.
>>
>>> However, it's also an information disclosure in various ways. The
>>> arguments of a program are often more sensitive than the name, and logs
>>> have a tendency to end up in various places, such as bug reports.
>>>
>>> An example of how this can be an issue:
>>> - You receive an email or other message with a sensitive link to follow
>>> - You open the link, which launches `firefox https://...`
>>> - You continue browsing from that window
>>> - Firefox crashes (and recovers and restarts, so you don't think
>>>   anything of it)
>>> - Later, you report a bug on a different piece of software, and the bug
>>>   reporting process includes a copy of the kernel log
>>
>> Yes, that's a possible way how such information can leak.
>>
>>> I am *not* saying that we shouldn't do this; it seems quite helpful.
>>> However, I think we need to arrange to treat this as sensitive
>>> information, similar to kptr_restrict.
>>
>> I wonder what the best solution could be.
>>
>> A somewhat trivial solution is to combine it with the dmesg_restrict sysctl, e.g.:
>>
>> * When ``dmesg_restrict`` is set to 0 there are no restrictions for users to read
>> dmesg. In this case my patch would limit the information (based on example above):
>>     cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 [note: other parameters hidden due to dmesg_restrict=0 sysctl]
>> So it would show the full argv[0] with a hint that people would need to change dmesg_restrict.
>>
>> * When ``dmesg_restrict`` is set to 1, users must have ``CAP_SYSLOG`` to use dmesg(8)
>> and the patch could output all parameters:
>>      cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu ....
>>
>> That would of course still leave few possible corner-cases where information
>> could leak, but since usually programs shouldn't crash and that
>> people usually shouldn't put sensitive information into the parameter
>> list directly, it's somewhat unlikely to happen.
>>
>> Another different solution would be to add another sysctl.
>>
>> Any other ideas?
>
> I don't think we should overload the meaning of dmesg_restrict. But
> overloading kptr_restrict seems reasonable to me. (Including respecting
> kptr_restrict==2 by not showing this at all.)

I'm fine with kptr_restrict, but I'm puzzled for which value of kptr_restrict
the command line should be shown then.
By looking at the meaning of kptr_restrict, I think the command line should be
hidden for values 0-2.
Do you suggest to add a new value "3" or am I missing something?

Helge

Re: [PATCH 0/3] Dump command line of faulting process to syslog

[Josh Triplett]

On Thu, Aug 04, 2022 at 10:39:52AM +0200, Helge Deller wrote:
> On 8/4/22 10:09, Josh Triplett wrote:
> > On Tue, Aug 02, 2022 at 09:40:50PM +0200, Helge Deller wrote:
> >> On 8/1/22 18:57, Josh Triplett wrote:
> >>> However, it's also an information disclosure in various ways. The
> >>> arguments of a program are often more sensitive than the name, and logs
> >>> have a tendency to end up in various places, such as bug reports.
> >>>
> >>> An example of how this can be an issue:
> >>> - You receive an email or other message with a sensitive link to follow
> >>> - You open the link, which launches `firefox https://...`
> >>> - You continue browsing from that window
> >>> - Firefox crashes (and recovers and restarts, so you don't think
> >>>   anything of it)
> >>> - Later, you report a bug on a different piece of software, and the bug
> >>>   reporting process includes a copy of the kernel log
> >>
> >> Yes, that's a possible way how such information can leak.
> >>
> >>> I am *not* saying that we shouldn't do this; it seems quite helpful.
> >>> However, I think we need to arrange to treat this as sensitive
> >>> information, similar to kptr_restrict.
[...]
> > I don't think we should overload the meaning of dmesg_restrict. But
> > overloading kptr_restrict seems reasonable to me. (Including respecting
> > kptr_restrict==2 by not showing this at all.)
> 
> I'm fine with kptr_restrict, but I'm puzzled for which value of kptr_restrict
> the command line should be shown then.
> By looking at the meaning of kptr_restrict, I think the command line should be
> hidden for values 0-2.
> Do you suggest to add a new value "3" or am I missing something?

I'm suggesting treating it the same as a pointer value:

0: always show command line
1: show command line if read by privileged caller
2: never show command line

That could either use kptr_restrict or use a separate cmdline_restrict.

Re: [PATCH 1/3] proc: Add get_task_cmdline_kernel() function

[kernel test robot]

Hi Helge,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/mm]
[also build test WARNING on linus/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Helge-Deller/Dump-command-line-of-faulting-process-to-syslog/20220801-232209
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 8f1d56f64f8d6b80dea2d1978d10071132a695c5
config: hexagon-randconfig-r005-20220801 (https://download.01.org/0day-ci/archive/20220807/202208072219.zrLROtui-lkp@intel.com/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 52cd00cabf479aa7eb6dbb063b7ba41ea57bce9e)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/82149c3c87cadf0a362c3015753f97c7582f31e4
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Helge-Deller/Dump-command-line-of-faulting-process-to-syslog/20220801-232209
        git checkout 82149c3c87cadf0a362c3015753f97c7582f31e4
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash kernel/sched/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from kernel/sched/core.c:84:
   In file included from kernel/sched/sched.h:50:
>> include/linux/proc_fs.h:222:64: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                  ^
   include/linux/proc_fs.h:222:72: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                          ^
   include/linux/proc_fs.h:222:80: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                                  ^
   kernel/sched/core.c:3577:20: warning: unused function 'rq_has_pinned_tasks' [-Wunused-function]
   static inline bool rq_has_pinned_tasks(struct rq *rq)
                      ^
   kernel/sched/core.c:5505:20: warning: unused function 'sched_tick_start' [-Wunused-function]
   static inline void sched_tick_start(int cpu) { }
                      ^
   kernel/sched/core.c:5506:20: warning: unused function 'sched_tick_stop' [-Wunused-function]
   static inline void sched_tick_stop(int cpu) { }
                      ^
   kernel/sched/core.c:6200:20: warning: unused function 'sched_core_cpu_starting' [-Wunused-function]
   static inline void sched_core_cpu_starting(unsigned int cpu) {}
                      ^
   kernel/sched/core.c:6201:20: warning: unused function 'sched_core_cpu_deactivate' [-Wunused-function]
   static inline void sched_core_cpu_deactivate(unsigned int cpu) {}
                      ^
   kernel/sched/core.c:6202:20: warning: unused function 'sched_core_cpu_dying' [-Wunused-function]
   static inline void sched_core_cpu_dying(unsigned int cpu) {}
                      ^
   9 warnings generated.
--
   In file included from kernel/sched/fair.c:54:
   In file included from kernel/sched/sched.h:50:
>> include/linux/proc_fs.h:222:64: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                  ^
   include/linux/proc_fs.h:222:72: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                          ^
   include/linux/proc_fs.h:222:80: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                                  ^
   kernel/sched/fair.c:5512:6: warning: no previous prototype for function 'init_cfs_bandwidth' [-Wmissing-prototypes]
   void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
        ^
   kernel/sched/fair.c:5512:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void init_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
   ^
   static 
   kernel/sched/fair.c:11697:6: warning: no previous prototype for function 'free_fair_sched_group' [-Wmissing-prototypes]
   void free_fair_sched_group(struct task_group *tg) { }
        ^
   kernel/sched/fair.c:11697:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void free_fair_sched_group(struct task_group *tg) { }
   ^
   static 
   kernel/sched/fair.c:11699:5: warning: no previous prototype for function 'alloc_fair_sched_group' [-Wmissing-prototypes]
   int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
       ^
   kernel/sched/fair.c:11699:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int alloc_fair_sched_group(struct task_group *tg, struct task_group *parent)
   ^
   static 
   kernel/sched/fair.c:11704:6: warning: no previous prototype for function 'online_fair_sched_group' [-Wmissing-prototypes]
   void online_fair_sched_group(struct task_group *tg) { }
        ^
   kernel/sched/fair.c:11704:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void online_fair_sched_group(struct task_group *tg) { }
   ^
   static 
   kernel/sched/fair.c:11706:6: warning: no previous prototype for function 'unregister_fair_sched_group' [-Wmissing-prototypes]
   void unregister_fair_sched_group(struct task_group *tg) { }
        ^
   kernel/sched/fair.c:11706:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void unregister_fair_sched_group(struct task_group *tg) { }
   ^
   static 
   kernel/sched/fair.c:489:20: warning: unused function 'list_del_leaf_cfs_rq' [-Wunused-function]
   static inline void list_del_leaf_cfs_rq(struct cfs_rq *cfs_rq)
                      ^
   kernel/sched/fair.c:510:19: warning: unused function 'tg_is_idle' [-Wunused-function]
   static inline int tg_is_idle(struct task_group *tg)
                     ^
   kernel/sched/fair.c:2961:20: warning: unused function 'account_numa_enqueue' [-Wunused-function]
   static inline void account_numa_enqueue(struct rq *rq, struct task_struct *p)
                      ^
   kernel/sched/fair.c:2965:20: warning: unused function 'account_numa_dequeue' [-Wunused-function]
   static inline void account_numa_dequeue(struct rq *rq, struct task_struct *p)
                      ^
   kernel/sched/fair.c:2969:20: warning: unused function 'update_scan_period' [-Wunused-function]
   static inline void update_scan_period(struct task_struct *p, int new_cpu)
                      ^
   kernel/sched/fair.c:4171:20: warning: unused function 'cfs_rq_is_decayed' [-Wunused-function]
   static inline bool cfs_rq_is_decayed(struct cfs_rq *cfs_rq)
                      ^
   kernel/sched/fair.c:4185:20: warning: unused function 'remove_entity_load_avg' [-Wunused-function]
   static inline void remove_entity_load_avg(struct sched_entity *se) {}
                      ^
   kernel/sched/fair.c:5493:20: warning: unused function 'sync_throttle' [-Wunused-function]
   static inline void sync_throttle(struct task_group *tg, int cpu) {}
                      ^
   kernel/sched/fair.c:5506:19: warning: unused function 'throttled_lb_pair' [-Wunused-function]
   static inline int throttled_lb_pair(struct task_group *tg,
                     ^
   kernel/sched/fair.c:5518:37: warning: unused function 'tg_cfs_bandwidth' [-Wunused-function]
   static inline struct cfs_bandwidth *tg_cfs_bandwidth(struct task_group *tg)
                                       ^
   kernel/sched/fair.c:5522:20: warning: unused function 'destroy_cfs_bandwidth' [-Wunused-function]
   static inline void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
                      ^
   kernel/sched/fair.c:5523:20: warning: unused function 'update_runtime_enabled' [-Wunused-function]
   static inline void update_runtime_enabled(struct rq *rq) {}
                      ^
   kernel/sched/fair.c:5524:20: warning: unused function 'unthrottle_offline_cfs_rqs' [-Wunused-function]
   static inline void unthrottle_offline_cfs_rqs(struct rq *rq) {}
                      ^
   21 warnings generated.
--
   In file included from kernel/sched/build_policy.c:34:
   In file included from kernel/sched/sched.h:50:
>> include/linux/proc_fs.h:222:64: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                  ^
   include/linux/proc_fs.h:222:72: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                          ^
   include/linux/proc_fs.h:222:80: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                                  ^
   In file included from kernel/sched/build_policy.c:45:
   kernel/sched/rt.c:259:6: warning: no previous prototype for function 'unregister_rt_sched_group' [-Wmissing-prototypes]
   void unregister_rt_sched_group(struct task_group *tg) { }
        ^
   kernel/sched/rt.c:259:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void unregister_rt_sched_group(struct task_group *tg) { }
   ^
   static 
   kernel/sched/rt.c:261:6: warning: no previous prototype for function 'free_rt_sched_group' [-Wmissing-prototypes]
   void free_rt_sched_group(struct task_group *tg) { }
        ^
   kernel/sched/rt.c:261:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   void free_rt_sched_group(struct task_group *tg) { }
   ^
   static 
   kernel/sched/rt.c:263:5: warning: no previous prototype for function 'alloc_rt_sched_group' [-Wmissing-prototypes]
   int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
       ^
   kernel/sched/rt.c:263:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int alloc_rt_sched_group(struct task_group *tg, struct task_group *parent)
   ^
   static 
   6 warnings generated.
--
   In file included from kernel/sched/build_utility.c:35:
>> include/linux/proc_fs.h:222:64: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                  ^
   include/linux/proc_fs.h:222:72: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                          ^
   include/linux/proc_fs.h:222:80: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
   static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
                                                                                  ^
   3 warnings generated.


vim +222 include/linux/proc_fs.h

   221	
 > 222	static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
   223	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 1/3] proc: Add get_task_cmdline_kernel() function

[Helge Deller]

On 8/7/22 16:28, kernel test robot wrote:
> Hi Helge,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on tip/x86/mm]
> [also build test WARNING on linus/master]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> [...]
>    include/linux/proc_fs.h:222:72: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
>    static inline void get_task_cmdline_kernel(struct task_struct *, char *, size_t) { }
>                                                                           ^
Will fix.

Thanks!
Helge