From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 B3AFA28D84F for ; Fri, 14 Nov 2025 16:52:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763139160; cv=none; b=aaO52zfqHnEfOf/WvFG1P3IDhz28LkNiODTJ95g+aNg/aGSsyOqabLWm/1/W+MK7EKRTuU3hC4f/fROp/qE1SnRjJe0S0a3HMgCzz7g8tEt6Xx60X4/8ZJ7z04023nqEuspTlmkOz4cTrNhnbHP1BEHkUCkX7mTVcWFMrZ/hapU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763139160; c=relaxed/simple; bh=ukDtEMQ3h+HhdTJApwHm9gtvXizGx7c4PKt+RTNGPfM=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=n+TiaHFzUTTeILJY+xvoY+zJTE2t7M/eJMb6ynHxoY5H1kOR72wwT3txw1ch9rWN/pcD4zCMO22L/rrhnOsCxP10tmUagppYfb0+lMy9iXA9gP/InraxYvgOtcVArdy+58gEJIKWFnkGlgtyEMBFfvoDOJf1yQ7ZmxW+lilVKj4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=dYUKLJgJ; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="dYUKLJgJ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A32DBC116B1; Fri, 14 Nov 2025 16:52:38 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1763139160; bh=ukDtEMQ3h+HhdTJApwHm9gtvXizGx7c4PKt+RTNGPfM=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=dYUKLJgJtebr3v2RLEJLGjWo41D48EJ74YaRNQVVcm9EIEADqqQMMJSdMPnkQyRAY XkyMOmFO3ICqJO7e5Nva3u3qleGxnEKKidN2/N8S3rc1/Nh30kccNHXjEAVPZsadOg dOiaDloor0qs0YW7KtSIGIMWfqCor3kC3qbk4otQ0YiPgFGwSZCynzXIoJUG4QHN/E IH4UrA/zU3x42MbWyJqcj2PmtkLUek9tZM/TDt1nf1GFO6ZuOKoUV7X2U3r7+ihKBA rPf51GRurGrGeTA6XskxG28Z/YZzJAQOnxxY5Dqu65vEFiDcfcKcHdWBmZsUeVhRK3 GxVPiDNBhTPRg== From: Pratyush Yadav To: Pasha Tatashin Cc: akpm@linux-foundation.org, bhe@redhat.com, rppt@kernel.org, jasonmiu@google.com, arnd@arndb.de, coxu@redhat.com, dave@vasilevsky.ca, ebiggers@google.com, graf@amazon.com, kees@kernel.org, linux-kernel@vger.kernel.org, kexec@lists.infradead.org, linux-mm@kvack.org Subject: Re: [PATCH v1 04/13] kho: Verify deserialization status and fix FDT alignment access In-Reply-To: <20251114155358.2884014-5-pasha.tatashin@soleen.com> (Pasha Tatashin's message of "Fri, 14 Nov 2025 10:53:49 -0500") References: <20251114155358.2884014-1-pasha.tatashin@soleen.com> <20251114155358.2884014-5-pasha.tatashin@soleen.com> Date: Fri, 14 Nov 2025 17:52:37 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Fri, Nov 14 2025, Pasha Tatashin wrote: > During boot, kho_restore_folio() relies on the memory map having been > successfully deserialized. If deserialization fails or no map is present, > attempting to restore the FDT folio is unsafe. > > Update kho_mem_deserialize() to return a boolean indicating success. Use > this return value in kho_memory_init() to disable KHO if deserialization > fails. Also, the incoming FDT folio is never used, there is no reason to > restore it. > > Additionally, use memcpy() to retrieve the memory map pointer from the FDT. > FDT properties are not guaranteed to be naturally aligned, and accessing > a 64-bit value via a pointer that is only 32-bit aligned can cause faults. > > Signed-off-by: Pasha Tatashin > --- > kernel/liveupdate/kexec_handover.c | 32 ++++++++++++++++++------------ > 1 file changed, 19 insertions(+), 13 deletions(-) > > diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c > index a4b33ca79246..83aca3b4af15 100644 > --- a/kernel/liveupdate/kexec_handover.c > +++ b/kernel/liveupdate/kexec_handover.c > @@ -450,20 +450,28 @@ static void __init deserialize_bitmap(unsigned int order, > } > } > > -static void __init kho_mem_deserialize(const void *fdt) > +/* Return true if memory was deserizlied */ > +static bool __init kho_mem_deserialize(const void *fdt) > { > struct khoser_mem_chunk *chunk; > - const phys_addr_t *mem; > + const void *mem_ptr; > + u64 mem; > int len; > > - mem = fdt_getprop(fdt, 0, PROP_PRESERVED_MEMORY_MAP, &len); > - > - if (!mem || len != sizeof(*mem)) { > + mem_ptr = fdt_getprop(fdt, 0, PROP_PRESERVED_MEMORY_MAP, &len); > + if (!mem_ptr || len != sizeof(u64)) { > pr_err("failed to get preserved memory bitmaps\n"); > - return; > + return false; > } > + /* FDT guarantees 32-bit alignment, have to use memcpy */ > + memcpy(&mem, mem_ptr, len); Perhaps get_unaligned(mem) would have been simpler? > + > + chunk = mem ? phys_to_virt(mem) : NULL; > + > + /* No preserved physical pages were passed, no deserialization */ > + if (!chunk) > + return false; Should we disallow all kho_restore_{folio,pages}() calls too if this fails? Ideally those should never happen since kho_retrieve_subtree() will fail, so maybe as a debug aid? > > - chunk = *mem ? phys_to_virt(*mem) : NULL; > while (chunk) { > unsigned int i; > > @@ -472,6 +480,8 @@ static void __init kho_mem_deserialize(const void *fdt) > &chunk->bitmaps[i]); > chunk = KHOSER_LOAD_PTR(chunk->hdr.next); > } > + > + return true; > } > > /* > @@ -1377,16 +1387,12 @@ static void __init kho_release_scratch(void) > > void __init kho_memory_init(void) > { > - struct folio *folio; > - > if (kho_in.scratch_phys) { > kho_scratch = phys_to_virt(kho_in.scratch_phys); > kho_release_scratch(); > > - kho_mem_deserialize(kho_get_fdt()); > - folio = kho_restore_folio(kho_in.fdt_phys); > - if (!folio) > - pr_warn("failed to restore folio for KHO fdt\n"); > + if (!kho_mem_deserialize(kho_get_fdt())) > + kho_in.fdt_phys = 0; The folio restore does serve a purpose: it accounts for that folio in the system's total memory. See the call to adjust_managed_page_count() in kho_restore_page(). In practice, I don't think it makes much of a difference, but I don't see why not. > } else { > kho_reserve_scratch(); > } -- Regards, Pratyush Yadav