Date: Mon, 17 Nov 2025 18:33:09 GMT From: Jilles Tjoelker <jilles@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 75a6c38e4d5c - main - sh: Fix a double free in a rare scenario with pipes Message-ID: <202511171833.5AHIX9Cr033109@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by jilles: URL: https://cgit.FreeBSD.org/src/commit/?id=75a6c38e4d5c651b7398bf2bea5baa41a0939e92 commit 75a6c38e4d5c651b7398bf2bea5baa41a0939e92 Author: Jilles Tjoelker <jilles@FreeBSD.org> AuthorDate: 2025-11-15 16:43:03 +0000 Commit: Jilles Tjoelker <jilles@FreeBSD.org> CommitDate: 2025-11-17 18:32:38 +0000 sh: Fix a double free in a rare scenario with pipes The command sh -c 'sleep 3 | sleep 2 & sleep 3 & kill %1; wait %1' crashes (with appropriate sanitization such as putting MALLOC_CONF=abort:true,junk:true in the environment or compiling with -fsanitize=address). What happens here is that waitcmdloop() calls dowait() with a NULL job pointer, instructing dowait() to freejob() if it's a non-interactive shell and $! was not and cannot be referenced for it. However, waitcmdloop() then uses fields possibly freed by freejob() and calls freejob() again. This only occurs if the job being waited for is identified via % syntax ($! has never been referenced for it), it is a pipeline with two or more elements and another background job has been started before the wait command. That seems special enough for a bug to remain. Test scripts written by Jilles would almost always use $! and not % syntax. We can instead make waitcmdloop() pass its job pointer to dowait(), fixing up things for that (waitcmdloop() will have to call deljob() if it does not call freejob()). The crash from https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=290330#c2 appears to be the same bug. PR: 290330 Reported by: bdrewery Reviewed by: bdrewery Differential Revision: https://reviews.freebsd.org/D53773 --- bin/sh/jobs.c | 3 ++- bin/sh/tests/builtins/Makefile | 1 + bin/sh/tests/builtins/wait11.0 | 6 ++++++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/bin/sh/jobs.c b/bin/sh/jobs.c index 1328ae50edef..0aaff5e1e140 100644 --- a/bin/sh/jobs.c +++ b/bin/sh/jobs.c @@ -573,6 +573,7 @@ waitcmdloop(struct job *job) freejob(job); else { job->remembered = 0; + deljob(job); if (job == bgjob) bgjob = NULL; } @@ -599,7 +600,7 @@ waitcmdloop(struct job *job) break; } } - } while (dowait(DOWAIT_BLOCK | DOWAIT_SIG, (struct job *)NULL) != -1); + } while (dowait(DOWAIT_BLOCK | DOWAIT_SIG, job) != -1); sig = pendingsig_waitcmd; pendingsig_waitcmd = 0; diff --git a/bin/sh/tests/builtins/Makefile b/bin/sh/tests/builtins/Makefile index 7fdecb23c817..407d2aeaa061 100644 --- a/bin/sh/tests/builtins/Makefile +++ b/bin/sh/tests/builtins/Makefile @@ -188,5 +188,6 @@ ${PACKAGE}FILES+= wait7.0 ${PACKAGE}FILES+= wait8.0 ${PACKAGE}FILES+= wait9.127 ${PACKAGE}FILES+= wait10.0 +${PACKAGE}FILES+= wait11.0 .include <bsd.test.mk> diff --git a/bin/sh/tests/builtins/wait11.0 b/bin/sh/tests/builtins/wait11.0 new file mode 100644 index 000000000000..d5fab26fb677 --- /dev/null +++ b/bin/sh/tests/builtins/wait11.0 @@ -0,0 +1,6 @@ +sleep 3 | sleep 2 & +sleep 3 & +kill %1 +wait %1 +r=$? +[ "$r" -gt 128 ] && [ "$(kill -l "$r")" = TERM ]help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202511171833.5AHIX9Cr033109>