Message-ID: <mi4tihk4dbncn7belrhp6ooudhpw4vdggerktu5333w3gqf3uf@vqlhc3y667mg>
Date: Tue, 2 Dec 2025 15:01:17 -0800
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: David Laight <david.laight.linux@...il.com>
Cc: Ingo Molnar <mingo@...nel.org>, x86@...nel.org, 
	linux-kernel@...r.kernel.org, Nathan Chancellor <nathan@...nel.org>, 
	Peter Zijlstra <peterz@...radead.org>, Alexandre Chartre <alexandre.chartre@...cle.com>
Subject: Re: [PATCH] objtool: Fix stack overflow in validate_branch()

On Tue, Dec 02, 2025 at 10:05:40PM +0000, David Laight wrote:
> On Tue, 2 Dec 2025 21:20:55 +0100
> Ingo Molnar <mingo@...nel.org> wrote:
> 
> > * Josh Poimboeuf <jpoimboe@...nel.org> wrote:
> > 
> > > On Tue, Dec 02, 2025 at 09:11:50AM -0800, Josh Poimboeuf wrote:  
> > > > > > > That's weird - how can a user-space tool run into stack 
> > > > > > > limits, are they set particularly conservatively?  
> > > > > > 
> > > > > > On my Fedora system, "ulimit -s" is 8MB.  You'd think that would be
> > > > > > enough :-)
> > > > > > 
> > > > > > In this case, objtool had over 20,000 stack frames caused by recursively
> > > > > > following over 7,000(!) conditional jumps in a single function.  
> > > > > 
> > > > > Ouch ...
> > > > > 
> > > > > ... which means that very likely we'll run into this problem again. :-/
> > > > > 
> > > > > Time to add stack overflow self-detection?
> > > > > 
> > > > > I've attached a simple proof-of-concept that uses 
> > > > > sigaltstacks based SIGSEGV handler to catch a stack 
> > > > > overflow:
> > > > > 
> > > > >   starship:/s/stack-overflow> ./overflow 
> > > > >   # Starting stack recursion:
> > > > > 
> > > > >   # WARNING: SIGSEGV received: Possible stack overflow detected!
> > > > > 
> > > > >   starship:/s/stack-overflow> 
> > > > > 
> > > > > Could we add something like this to objtool, with 
> > > > > perhaps a look at the interrupted stack pointer from 
> > > > > SIGSEGV_handler(), to make sure the SIGSEGV was due to 
> > > > > a stack overflow?  
> > > > 
> > > > Yes, I think that would be wise.  I've been thinking objtool could use a
> > > > SIGSEGV handler anyway, as it crashes more often than one would hope,
> > > > with a cryptic non-helpful error message for the user.
> > > > 
> > > > I'll work on it.  
> > > 
> > > Is something like the below sufficient?  Or do you think we should add
> > > logic to distinguish the stack overflow from other crashes?
> > > 
> > > ---8<---
> > > 
> > > From: Josh Poimboeuf <jpoimboe@...nel.org>
> > > Subject: [PATCH] objtool: Improve error message for SIGSEGV
> > > 
> > > When the kernel build fails due to an objtool seg fault, the error
> > > message is confusing:
> > > 
> > >   make[5]: *** [scripts/Makefile.build:503: drivers/scsi/qla2xxx/qla2xxx.o] Error 139
> > >   make[5]: *** Deleting file 'drivers/scsi/qla2xxx/qla2xxx.o'
> > >   make[4]: *** [scripts/Makefile.build:556: drivers/scsi/qla2xxx] Error 2
> > >   make[3]: *** [scripts/Makefile.build:556: drivers/scsi] Error 2
> > >   make[2]: *** [scripts/Makefile.build:556: drivers] Error 2
> > >   make[1]: *** [/home/jpoimboe/git/linux/Makefile:2013: .] Error 2
> > >   make: *** [Makefile:248: __sub-make] Error 2
> > > 
> > > Add a signal handler which prints an error message like:
> > > 
> > >   drivers/scsi/qla2xxx/qla2xxx.o: error: objtool: SIGSEGV (Segmentation Fault) received at address 0x7ffc5f33bf30
> > > 
> > > ... and re-raises the signal so the core dump still gets triggered.  
> > 
> > Could we somehow determine that 0x7ffc5f33bf30 is off 
> > the end of the stack or so and that this is a stack 
> > overflow?
> 
> You could compare it to the address of something on-stack during program startup.
> Probably even argv[] - isn't that always at the bottom of the stack?
> If you read the rlimit value, maybe the recursive loop could abort
> before the fault.

Here's with reading /proc/self/maps and rlimit, it's not too bad:

From: Josh Poimboeuf <jpoimboe@...nel.org>
Subject: [PATCH] objtool: Add signal error handling

When the kernel build fails due to an objtool seg fault, the error
message is confusing:

  make[5]: *** [scripts/Makefile.build:503: drivers/scsi/qla2xxx/qla2xxx.o] Error 139
  make[5]: *** Deleting file 'drivers/scsi/qla2xxx/qla2xxx.o'
  make[4]: *** [scripts/Makefile.build:556: drivers/scsi/qla2xxx] Error 2
  make[3]: *** [scripts/Makefile.build:556: drivers/scsi] Error 2
  make[2]: *** [scripts/Makefile.build:556: drivers] Error 2
  make[1]: *** [/home/jpoimboe/git/linux/Makefile:2013: .] Error 2
  make: *** [Makefile:248: __sub-make] Error 2

Add a signal handler which prints an error message like:

  drivers/scsi/qla2xxx/qla2xxx.o: error: SIGSEGV: objtool stack overflow!

or

  drivers/scsi/qla2xxx/qla2xxx.o: error: SIGSEGV: objtool crash!

Also, re-raise the signal so the core dump still gets triggered.

Suggested-by: Ingo Molnar <mingo@...nel.org>
Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
---
 tools/objtool/Build                     |   1 +
 tools/objtool/include/objtool/objtool.h |   2 +
 tools/objtool/objtool.c                 |   4 +-
 tools/objtool/signal.c                  | 138 ++++++++++++++++++++++++
 4 files changed, 144 insertions(+), 1 deletion(-)
 create mode 100644 tools/objtool/signal.c

diff --git a/tools/objtool/Build b/tools/objtool/Build
index 9982e665d58d..600da051af12 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -18,6 +18,7 @@ objtool-y += libstring.o
 objtool-y += libctype.o
 objtool-y += str_error_r.o
 objtool-y += librbtree.o
+objtool-y += signal.o
 
 $(OUTPUT)libstring.o: ../lib/string.c FORCE
 	$(call rule_mkdir)
diff --git a/tools/objtool/include/objtool/objtool.h b/tools/objtool/include/objtool/objtool.h
index f7051bbe0bcb..6dc12a59ad00 100644
--- a/tools/objtool/include/objtool/objtool.h
+++ b/tools/objtool/include/objtool/objtool.h
@@ -41,6 +41,8 @@ struct objtool_file {
 
 char *top_level_dir(const char *file);
 
+int init_signal_handler(void);
+
 struct objtool_file *objtool_open_read(const char *_objname);
 
 int objtool_pv_add(struct objtool_file *file, int idx, struct symbol *func);
diff --git a/tools/objtool/objtool.c b/tools/objtool/objtool.c
index 3c26ed561c7e..1c3622117c33 100644
--- a/tools/objtool/objtool.c
+++ b/tools/objtool/objtool.c
@@ -104,11 +104,13 @@ char *top_level_dir(const char *file)
 	return str;
 }
 
-
 int main(int argc, const char **argv)
 {
 	static const char *UNUSED = "OBJTOOL_NOT_IMPLEMENTED";
 
+	if (init_signal_handler())
+		return -1;
+
 	/* libsubcmd init */
 	exec_cmd_init("objtool", UNUSED, UNUSED, UNUSED);
 	pager_init(UNUSED);
diff --git a/tools/objtool/signal.c b/tools/objtool/signal.c
new file mode 100644
index 000000000000..ce6605d53044
--- /dev/null
+++ b/tools/objtool/signal.c
@@ -0,0 +1,138 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <unistd.h>
+#include <sys/resource.h>
+#include <string.h>
+
+#include <objtool/objtool.h>
+#include <objtool/warn.h>
+
+static unsigned long stack_limit;
+
+static bool is_stack_overflow(void *fault_addr)
+{
+	unsigned long fault = (unsigned long)fault_addr;
+
+	/* Check if fault is in the guard page just below the limit. */
+	return fault < stack_limit && fault >= stack_limit - 4096;
+}
+
+static void signal_handler(int sig_num, siginfo_t *info, void *context)
+{
+	struct sigaction sa_dfl = {0};
+	const char *sig_name;
+	char msg[256];
+	int msg_len;
+
+	switch (sig_num) {
+	case SIGSEGV:
+		sig_name = "SIGSEGV";
+		break;
+	case SIGBUS:
+		sig_name = "SIGBUS";
+		break;
+	case SIGILL:
+		sig_name = "SIGILL";
+		break;
+	case SIGABRT:
+		sig_name = "SIGABRT";
+		break;
+	default:
+		sig_name = "Unknown signal";
+		break;
+	}
+
+	if (is_stack_overflow(info->si_addr)) {
+		msg_len = snprintf(msg, sizeof(msg),
+				   "%s: error: %s: objtool stack overflow!\n",
+				   objname, sig_name);
+	} else {
+		msg_len = snprintf(msg, sizeof(msg),
+				   "%s: error: %s: objtool crash!\n",
+				   objname, sig_name);
+	}
+
+	write(STDERR_FILENO, msg, msg_len);
+
+	/* Re-raise the signal to trigger the core dump */
+	sa_dfl.sa_handler = SIG_DFL;
+	sigaction(sig_num, &sa_dfl, NULL);
+	raise(sig_num);
+}
+
+static int read_stack_limit(void)
+{
+	unsigned long stack_start, stack_end;
+	struct rlimit rlim;
+	char line[256];
+	int ret = 0;
+	FILE *fp;
+
+	if (getrlimit(RLIMIT_STACK, &rlim)) {
+		ERROR_GLIBC("getrlimit");
+		return -1;
+	}
+
+	fp = fopen("/proc/self/maps", "r");
+	if (!fp) {
+		ERROR_GLIBC("fopen");
+		return -1;
+	}
+
+	while (fgets(line, sizeof(line), fp)) {
+		if (strstr(line, "[stack]")) {
+			if (sscanf(line, "%lx-%lx", &stack_start, &stack_end) != 2) {
+				ERROR_GLIBC("sscanf");
+				ret = -1;
+				goto done;
+			}
+			stack_limit = stack_end - rlim.rlim_cur;
+			goto done;
+		}
+	}
+
+	ret = -1;
+	ERROR("/proc/self/maps: can't find [stack]");
+
+done:
+	fclose(fp);
+	return ret;
+}
+
+int init_signal_handler(void)
+{
+	int signals[] = {SIGSEGV, SIGBUS, SIGILL, SIGABRT};
+	struct sigaction sa;
+	stack_t ss;
+
+	if (read_stack_limit())
+		return -1;
+
+	ss.ss_sp = malloc(SIGSTKSZ);
+	if (!ss.ss_sp) {
+		ERROR_GLIBC("malloc");
+		return -1;
+	}
+	ss.ss_size = SIGSTKSZ;
+	ss.ss_flags = 0;
+
+	if (sigaltstack(&ss, NULL) == -1) {
+		ERROR_GLIBC("sigaltstack");
+		return -1;
+	}
+
+	sa.sa_sigaction = signal_handler;
+	sigemptyset(&sa.sa_mask);
+
+	sa.sa_flags = SA_ONSTACK | SA_SIGINFO;
+
+	for (int i = 0; i < ARRAY_SIZE(signals); i++) {
+		if (sigaction(signals[i], &sa, NULL) == -1) {
+			ERROR_GLIBC("sigaction");
+			return -1;
+		}
+	}
+
+	return 0;
+}
-- 
2.51.1

Powered by blists - more mailing lists