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 BC1523C2BA4 for ; Wed, 23 Sep 2026 23:24:57 +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=1790205898; cv=none; b=uNR8HdVqqbkPlUWgwUqcDkWDbBLBSh9VUvsyRI25KAxdCpixFtu7In3sG4aa0WBhC2hh3QidOYVKfv/I87CA2TVvjyhYzMzL9Jv88wsLG/QfSOIGkMoUQxZnvKbKEsMzEGcLLf5PUzZsHfVb03gskYgN340iglu0Vw5j8VVj9w0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1790205898; c=relaxed/simple; bh=YP+HaHZiSayNoB99gQgkKGP0qHM6YuP4LC/EvJtCTcI=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=NzWL5CM8klgPyUvPEddWSVbDp3KM/tazXd62wB5b9fxmIZdJFscB82Ml3dxU/+dQGwrLTHXtmaPJGTsu3k2mB3GsLCYRYzpRAo0LiehxmQZLi4HcuCqk70L7qGr1gLt+pcpG5Bk/SkP49ul5SCdEjeirsLm+RTBL+r+hZD1IcUM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=OMIRNr4O; 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="OMIRNr4O" Received: by smtp.kernel.org (Postfix) with ESMTPSA id BCC0C1F000FF; Wed, 23 Sep 2026 23:24:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1790205897; bh=U9YKFd/Hl7YhdOyAieKBjsie7xEHxxz4e55B/x6uzdo=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=OMIRNr4OLiUixLKnhBolSnExmjDFWiRUOgWv20SvZaXjb2auAnjVO7paMjuqxwVUh gAznIr7cxA8iwbMkmKDqKHfpe1TD8i+ljP6YUolouKAwsl4f7+f1rOlXQVB0XCdwbH zyqskPgnBxQTHf32dDgeBvVOID2t7ytyP4gz5du6Is0yMc5oUiSqdo18GwgRg6AGe4 ExTY00SKLiY9HVRr3LlDd7XhgF4nLqikcaON2hjaUoR0iYwBwr+xjA1FfSfDhUATqS W/bTglH0wjmNowS+kVve+Q8JaSPv3Vhw8YXbWFuO3I97nSsjIRd8+ETJemkQbLwLdK 92jaAVSIpl77A== From: sashiko-bot@kernel.org Subject: Re: [RFC PATCH 5/6] mm/memfd_luo: allow preserving a tmpfs file Reply-To: sashiko-reviews@lists.linux.dev To: "Pasha Tatashin" , "Mike Rapoport" , "Andrew Morton" , "Lorenzo Stoakes" , "Pratyush Yadav" , "Baolin Wang" , "Samiullah Khawaja" , "Hugh Dickins" , "David Hildenbrand" , "David Matlack" , "Alexander Graf" Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org In-Reply-To: <20260923224408.3745689-6-pratyush@kernel.org> References: <20260923224408.3745689-1-pratyush@kernel.org> <20260923224408.3745689-6-pratyush@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Wed, 23 Sep 2026 23:24:56 +0000 Message-Id: <20260923232456.BCC0C1F000FF@smtp.kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 4 potential issue(= s) to consider: - [High] mm: restored file permission bits are corrupted by current_umask() - [High] mm: TOCTOU race allows unprivileged userspace to disrupt tmpfs fil= e retrieve - [High] mm: memory leak of KHO folios on tmpfs_luo_file_retrieve failure - [Medium] mm: potential double free / UAF of ser and folios on successful = retrieve -- commit 4087da995ba7fe7684120abaf9ec59deb366b97b Author: Pratyush Yadav (Google) mm/memfd_luo: allow preserving a tmpfs file This commit introduces tmpfs file preservation across kexec live updates. It preserves regular files within a tmpfs mount by serializing their properties and memory folios, and restores them when the mount is retrieved. > diff --git a/mm/memfd_luo.c b/mm/memfd_luo.c > --- a/mm/memfd_luo.c > +++ b/mm/memfd_luo.c > @@ -1005,62 +1005,51 @@ static int tmpfs_luo_file_retrieve(struct liveupd= ate_file_op_args *args) [ ... ] > err =3D liveupdate_get_file_incoming(args->session, ser->mnt_token, > &mnt_file); > if (err) { > pr_err("failed to retrieve tmpfs mount: %pe\n", ERR_PTR(err)); > goto free_ser; > } [Severity: High] If this path jumps to the free_ser error label, does it permanently leak the physical memory reserved for the KHO folios? The free_ser error label frees the ser metadata, but does not discard the physical folios or the ser->folios vmalloc region. Because the function returns an error, tmpfs_luo_file_finish() may skip its cleanup logic if args->retrieve_status is set to a non-zero error code. This would permanent= ly leak the memory of all files preserved inside that mount. [ ... ] > /* > * TODO: This is racy. This will link the file in the mount so after > * this call userspace can already open the file and write to it. > * Ideally we should first create the inode and set it up, and only > * then link it to the root. > */ > file =3D file_open_root(&mnt_file->f_path, ser->name, > O_RDWR | O_CREAT | O_EXCL | O_LARGEFILE, > ser->mode); [Severity: High] Does this code inadvertently strip the file permissions via the orchestrator's umask? VFS creation functions like file_open_root(O_CREAT) will apply ~current_umask() to the newly created file, which could alter the preserved ser->mode. Since the kernel code lacks a subsequent chmod() or vfs_setattr() step to enforce the exact preserved mode, the restored file might have incorrect permission bits. [Severity: High] Does linking the file into the namespace here open a TOCTOU race window? Since file_open_root() immediately links the restored file into the tmpfs namespace, unprivileged userspace could potentially open and write or mmap the file before memfd_luo_retrieve_folios() is called. This would instantia= te page cache folios, causing the subsequent shmem_add_to_page_cache() inside memfd_luo_retrieve_folios() to hit an -EEXIST collision and permanently drop the preserved data folios. [ ... ] > args->file =3D file; > kho_restore_free(ser); >=20 > return 0; [Severity: Medium] Could this lead to a double-free or use-after-free regression of the KHO folios on a successful retrieve? This code frees the ser object but leaves args->serialized_data as a dangling pointer. [ ... ] > static void tmpfs_luo_file_finish(struct liveupdate_file_op_args *args) > { > struct memfd_luo_folio_ser *folios_ser; > struct tmpfs_luo_file_ser *ser; >=20 > /* > * A successful retrieve() already consumed the preserved memory, and a > * failed one cleaned up what it could. Only a file that was never > * retrieved is left to discard here. > */ > if (args->retrieve_status || !args->serialized_data) > return; [Severity: Medium] If the LUO core framework sets args->retrieve_status to 0 upon a successful return, tmpfs_luo_file_finish() might bypass this early return, dereference the dangling args->serialized_data pointer, and mistakenly double-free the folios. Could args->serialized_data be explicitly cleared after it is freed to prevent this? --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260923224408.3745= 689-1-pratyush@kernel.org?part=3D5