From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 845DE4A2630; Mon, 21 Sep 2026 13:46:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789998368; cv=none; b=S0Ykn0LLO7L6OE18AQ+Fs/xb61Flzxuy8gE7c4jUGqiGJK5J+Fd9R7kcOEGtowSjl1zORwy/bCOMjd9txHpNZNkRRN7lucQEnZsQWkRUAE93qrMWEi3PGZT92+skN5+JAyQgGpUL349jp5OI37ro/xrhDl8s2usUCx1ban9GHEM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789998368; c=relaxed/simple; bh=lj99++MiJpzbFK4wClDD2ih1T3hXCm3XRQc2cRY0Hok=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=OOWj+qceIMN/qAuHXbKWh/mqNkERzgN3eQV+PdBZQ1W9IHnc3Qyeo5kdSlzBGi16ZNtrPf6PJj+fa6ZvXN7zpdXMDWy592pSJotMGLMZCDlTgrqpT5+aLkHNGqZqGdl12c/Vb+MXl6FoO0LgtQ90wZ5BgcDR+20JYl0y0qfT1Zw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fqY+nvrE; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fqY+nvrE" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 352091F00898; Mon, 21 Sep 2026 13:46:03 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1789998367; bh=iAGZ6PU49QMZkoSdNCCgv05Mtfg8cEoYV92BD8cI+30=; h=From:Date:Subject:References:In-Reply-To:To:Cc; b=fqY+nvrEB5A3kmuky5pBIBfQfBbIM9TbNDC9pClgfbUCBHbJqnshrb3SA7ZZGGKIu L1Z4vbKkEzc35WOaTsdzqk9BBB6M8OJiPPMGm1Xg3p25+9cGjFX+XP/nQLCcy883/1 QZW3s3xH5pslUlITzgj03Hr1P4SHHiAAg7dDERsQRqLr95e+ZFE7P2QjPu/qV0SOCK yyD7ytnHBk/XHkz7Sj071IDmYXoOJjmIhNIq6gflzyPN3fbD6FnCNZjXkfbhZ+fZjE ldgVI2nOFjsTD8n8qikO5NlyHJvVl5/dEmMXmV5X6+4rTn0V/EzzR1ZiPkR61jpbjL 9yZ31tmAsfY0A== From: Christian Brauner Date: Mon, 21 Sep 2026 15:45:06 +0200 Subject: [PATCH v3 17/17] fs: close files from the highest descriptor down Precedence: bulk X-Mailing-List: linux-fsdevel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20260921-work-coredump-fixes-v3-17-8e4adb1619e6@kernel.org> References: <20260921-work-coredump-fixes-v3-0-8e4adb1619e6@kernel.org> In-Reply-To: <20260921-work-coredump-fixes-v3-0-8e4adb1619e6@kernel.org> To: Oleg Nesterov , Chris Mason , linux-fsdevel@vger.kernel.org Cc: Jens Axboe , Alexander Viro , Jan Kara , NeilBrown , Ingo Molnar , Peter Zijlstra , linux-mm@kvack.org, io-uring@vger.kernel.org, "Christian Brauner (Amutable)" X-Mailer: b4 0.17-dev-db0b7 X-Developer-Signature: v=1; a=openpgp-sha256; l=5081; i=brauner@kernel.org; h=from:subject:message-id; bh=lj99++MiJpzbFK4wClDD2ih1T3hXCm3XRQc2cRY0Hok=; b=owGbwMvMwCU28Zj0gdSKO4sYT6slMWRtNLnL9vDs5WVFOWaNX43Yha5uOhD6gfGj//blH3We8 yZGMz/b3FHKwiDGxSArpsji0G4SLrecp2KzUaYGzBxWJpAhDFycAjCRskUMPxmL+GKPTJ+SXnZp Pr/e5u283jvfb/t17MmH+QseXTF+nrKbkWGjsdXzOS4nbx733u0w9ZHLVGWfJw/3MYYlrHub8n/ i+RxeAA== X-Developer-Key: i=brauner@kernel.org; a=openpgp; fpr=4880B8C9BD0E5106FC070F4F7B3C391EFEA93624 close_files(), __range_close() and close_cloexec_files() walk the descriptor table from the lowest descriptor up. They used to call filp_close(), which left the final __fput() to task work. task work is a LIFO list, so the releases ran after the walk had finished and in the opposite direction, highest descriptor first. This dumb ordering is relevant for a bunch of broken but long-standing cases. It matters whenever the ->flush() or ->release() of one file waits for something that only the release of another file of the same table provides. Then one of the two orders deadlocks and the other one doesn't: exit, fd 3 is one end of a pipe peer ------------------------------- ---- splice(socket -> pipe) pipe_lock() waits for data or EOF close_files() fd 3: pipe_release() mutex_lock(&pipe->mutex) held by the peer fd 5: the socket, not reached would be the peer's EOF Programs create the thing that guards or wakes another thing first. Hence, it gets the lower descriptor which is the layout that breaks: (1) a tap device released before the AF_LLC socket that holds a reference to it (2) an unlinked fsdax file evicted before the pipe that holds its vmspliced pages (3) an overlayfs directory whose release queues up behind an unlink that waits for a splice into the same directory The exiting task is unkillable in all of them. All of that crap can obviously also become a bug if you reorder the file descriptors. Continue walking all three tables from the highest descriptor down. That restores the order the deferred puts had. ->flush() moves with the release. So it now runs highest descriptor first as well. None of this fixes the underlying defects. For every one of these pairs the mirrored layout deadlocked before and deadlocks again now: (1') splice() holding pipe->mutex across unbounded socket and tty I/O (2') AF_LLC keeping a netdev reference without a NETDEV_UNREGISTER handler (3') uninterruptible wait in dax_break_layout_final() (4') ovl_splice_write() sleeping under the inode lock It all predates the synchronous close and each should really get fixed. Reported-by: Chris Mason Signed-off-by: Christian Brauner (Amutable) --- fs/file.c | 55 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/fs/file.c b/fs/file.c index 76e328edf630..7f8d0afd8807 100644 --- a/fs/file.c +++ b/fs/file.c @@ -497,24 +497,21 @@ static struct fdtable *close_files(struct files_struct *files) * files structure. */ struct fdtable *fdt = rcu_dereference_raw(files->fdt); - unsigned int i, j = 0; + unsigned int j = fdt->max_fds / BITS_PER_LONG; + + /* Highest fd first, the order the deferred puts ran in. */ + while (j--) { + unsigned long set = fdt->open_fds[j]; - for (;;) { - unsigned long set; - i = j * BITS_PER_LONG; - if (i >= fdt->max_fds) - break; - set = fdt->open_fds[j++]; while (set) { - if (set & 1) { - struct file *file = fdt->fd[i]; - if (file) { - filp_close_sync(file, files); - cond_resched(); - } + unsigned int bit = __fls(set); + struct file *file = fdt->fd[j * BITS_PER_LONG + bit]; + + set ^= 1UL << bit; + if (file) { + filp_close_sync(file, files); + cond_resched(); } - i++; - set >>= 1; } } @@ -801,10 +798,14 @@ static inline void __range_close(struct files_struct *files, unsigned int fd, n = last_fd(fdt); max_fd = min(max_fd, n); - for (fd = find_next_bit(fdt->open_fds, max_fd + 1, fd); - fd <= max_fd; - fd = find_next_bit(fdt->open_fds, max_fd + 1, fd + 1)) { - file = file_close_fd_locked(files, fd); + /* Highest fd first, see close_files(). */ + for (n = max_fd + 1; n > fd; ) { + unsigned int cur = find_last_bit(fdt->open_fds, n); + + if (cur >= n || cur < fd) + break; + n = cur; + file = file_close_fd_locked(files, cur); if (file) { spin_unlock(&files->file_lock); filp_close_sync(file, files); @@ -908,20 +909,22 @@ void close_cloexec_files(struct files_struct *files) /* exec unshares first */ spin_lock(&files->file_lock); - for (i = 0; ; i++) { + fdt = files_fdtable(files); + /* Highest fd first, see close_files(). */ + for (i = fdt->max_fds / BITS_PER_LONG; i--; ) { unsigned long set; - unsigned fd = i * BITS_PER_LONG; + fdt = files_fdtable(files); - if (fd >= fdt->max_fds) - break; set = fdt->close_on_exec[i]; if (!set) continue; fdt->close_on_exec[i] = 0; - for ( ; set ; fd++, set >>= 1) { + while (set) { + unsigned int bit = __fls(set); + unsigned fd = i * BITS_PER_LONG + bit; struct file *file; - if (!(set & 1)) - continue; + + set ^= 1UL << bit; file = fdt->fd[fd]; if (!file) continue; -- 2.53.0