2022-08-08 13:27:47

by Helge Deller

[permalink] [raw]
Subject: [PATCH v3 0/4] Dump command line of faulting process to syslog

This patch series dumps the command line (including the program parameters) of
a faulting process to the syslog.

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.

For example, a dump on parisc shows:
do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]

-> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.
With this patch you will see that cc1 crashed while compiling some haskell code:

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

Another example are the glibc testcases which always segfault in "ld.so.1" with no other info:

do_page_fault() command='ld.so.1' type=15 address=0x565921d8 in libc.so[f7339000+1bb000]

-> With the patch you can see it was the "tst-safe-linking-malloc-hugetlb1" testcase:

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

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

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

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

The relevant changes are in patches #1 and #2.
Patch #3 adds the cmdline dump on x86.
Patch #4 drops code from arc which now becomes unnecessary as this is done by generic code.

Helge

---

Changes in v3:
- require task to be locked by caller, noticed by kernel test robot
- add parameter names in header files, noticed by kernel test robot

Changes in v2:
- Don't dump all or parts of the commandline depending on the
kptr_restrict sysctl value (suggested by Josh Triplett).
- Patch sent to more arch mailing lists

Helge Deller (4):
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
arc: Use generic dump_stack_print_cmdline() implementation

arch/arc/kernel/troubleshoot.c | 24 -----------
arch/x86/mm/fault.c | 2 +
fs/proc/base.c | 74 +++++++++++++++++++++++-----------
include/linux/printk.h | 5 +++
include/linux/proc_fs.h | 5 +++
lib/dump_stack.c | 34 ++++++++++++++++
6 files changed, 97 insertions(+), 47 deletions(-)

--
2.37.1


2022-08-08 13:31:21

by Helge Deller

[permalink] [raw]
Subject: [PATCH v3 3/4] x86/fault: Dump command line of faulting process to syslog

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

Signed-off-by: Helge Deller <[email protected]>
---
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

2022-08-08 13:32:19

by Helge Deller

[permalink] [raw]
Subject: [PATCH v3 2/4] lib/dump_stack: Add dump_stack_print_cmdline() and wire up in dump_stack_print_info()

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

Josh Triplett noted that dumping such command line parameters into
syslog may theoretically lead to information disclosure.
That's why this patch checks the value of the kptr_restrict sysctl
variable and will not print any information if kptr_restrict==2, and
will not show the program parameters if kptr_restrict==1.

Signed-off-by: Helge Deller <[email protected]>
---
include/linux/printk.h | 5 +++++
lib/dump_stack.c | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 39 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..38ef1067c7eb 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,37 @@ 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];
+
+ if (kptr_restrict >= 2)
+ return; /* never show command line */
+
+ /* get command line */
+ get_task_cmdline_kernel(current, cmdline, sizeof(cmdline));
+
+ if (kptr_restrict == 1) {
+ char *p;
+
+ /* if restricted show program path only */
+ p = strchr(cmdline, ' ');
+ if (p) {
+ *p = 0;
+ strlcat(cmdline,
+ " ... [parameters hidden due to kptr_restrict]",
+ 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 +94,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

2022-08-08 13:33:32

by Helge Deller

[permalink] [raw]
Subject: [PATCH v3 1/4] proc: Add get_task_cmdline_kernel() function

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 give additional debug info via the parameters
with which 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 <[email protected]>

--
Changes in v3:
- add parameter names in header files, noticed by: kernel test robot
- require task to be locked by caller
---
fs/proc/base.c | 74 ++++++++++++++++++++++++++++-------------
include/linux/proc_fs.h | 5 +++
2 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 8dfa36a99c74..e2d4152aed34 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,57 @@ 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.
+ * NOTE: Requires that the task was locked with task_lock(task) by the caller.
+ */
+void get_task_cmdline_kernel(struct task_struct *task,
+ char *cmdline, size_t maxcount)
+{
+ struct mm_struct *mm;
+ int i;
+
+ mm = task->mm;
+ if (!mm || (task->flags & PF_KTHREAD))
+ return;
+
+ memset(cmdline, 0, maxcount);
+ get_mm_cmdline(mm, 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..c802bc668656 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 *task,
+ 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 *task, char *cmdl, size_t m) { }
+
#endif /* CONFIG_PROC_FS */

struct net;
--
2.37.1