From: Will Deacon <will@kernel.org>
To: Ryan Roberts <ryan.roberts@arm.com>
Cc: Dev Jain <dev.jain@arm.com>,
catalin.marinas@arm.com, rppt@kernel.org,
shijie@os.amperecomputing.com, yang@os.amperecomputing.com,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] arm64/pageattr: Propagate return value from __change_memory_common
Date: Mon, 24 Nov 2025 19:10:23 +0000 [thread overview]
Message-ID: <aSStn55E-omGunwA@willie-the-truck> (raw)
In-Reply-To: <fa1def37-b5c6-4652-890d-cae9ab45f82d@arm.com>
On Mon, Nov 24, 2025 at 06:09:31PM +0000, Ryan Roberts wrote:
> On 24/11/2025 15:11, Will Deacon wrote:
> > On Thu, Nov 13, 2025 at 11:55:48AM +0000, Ryan Roberts wrote:
> >> On 12/11/2025 06:27, Dev Jain wrote:
> >>> The rodata=on security measure requires that any code path which does
> >>> vmalloc -> set_memory_ro/set_memory_rox must protect the linear map alias
> >>> too. Therefore, if such a call fails, we must abort set_memory_* and caller
> >>> must take appropriate action; currently we are suppressing the error, and
> >>> there is a real chance of such an error arising post commit a166563e7ec3
> >>> ("arm64: mm: support large block mapping when rodata=full"). Therefore,
> >>> propagate any error to the caller.
> >>>
> >>> Fixes: a166563e7ec3 ("arm64: mm: support large block mapping when rodata=full")
> >>> Signed-off-by: Dev Jain <dev.jain@arm.com>
> >>
> >> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
> >>
> >> It would be good to get this into v6.18 I guess?
> >
> > I'm not sure I see the urgency. When the commit message says:
> >
> > "there is a real chance of such an error arising post commit a166563e7ec3"
> >
> > afaict that's either due to -ENOMEM
>
> Yes this is the main risk; failing to allocate an intermediate page table during
> split_kernel_leaf_mapping(). I have no idea how likely that is in production.
> The only data point I have is that for the theoretical memory hotplug -ENOMEM
> that Chaitanya and Linu fixed recently, we tried to provoke it by hotplugging a
> lot of memory on a system under high memory pressure; no matter what we did, we
> couldn't get that 4K pgtable allocation to fail. So on that basis, I think we
> are unlikely to see it.
>
> > or some hideous issue with the
> > page-tables (e.g. the -EINVALs in pageattr_pXd_entry() seem completely
> > unnecessary to me).
>
> I thought the -EINVALs were trying to catch the case where someone tries to set
> permissions on a sub-portion of a vmalloc_huge() area on a system that doesn't
> support BBML2_NOABORT. But looking again, we already refuse vmalloc_huge in
> change_memory_common.
>
> >
> > Do you think failure is actually likely and recoverable?
>
> As above, I think failure is unlikely, but not impossible. I guess the result
> would be memory that remains RW when it should have been set RO or RX. But I
> think it will all work itself out at vfree. I think.
>
> We can defer this to next cycle if you prefer.
Yeah, I think we have bigger problems if we can't find memory to allocate
page tables tbh.
Will
next prev parent reply other threads:[~2025-11-24 19:10 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-12 6:27 [PATCH 0/2] arm64/mm: A fix and a documentation bit for change_memory_common() Dev Jain
2025-11-12 6:27 ` [PATCH 1/2] arm64/pageattr: Propagate return value from __change_memory_common Dev Jain
2025-11-12 22:27 ` Yang Shi
2025-11-19 18:06 ` Catalin Marinas
2025-11-19 19:33 ` Yang Shi
2025-11-13 11:55 ` Ryan Roberts
2025-11-13 17:38 ` Yang Shi
2025-11-24 15:11 ` Will Deacon
2025-11-24 18:09 ` Ryan Roberts
2025-11-24 19:10 ` Will Deacon [this message]
2025-11-19 18:03 ` Catalin Marinas
2025-11-20 4:10 ` Dev Jain
2025-11-12 6:27 ` [PATCH 2/2] arm64/mm: Document why linear map split failure upon vm_reset_perms is not problematic Dev Jain
2025-11-12 22:34 ` Yang Shi
2025-11-13 12:25 ` Ryan Roberts
2025-11-28 15:52 ` [PATCH 0/2] arm64/mm: A fix and a documentation bit for change_memory_common() Catalin Marinas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aSStn55E-omGunwA@willie-the-truck \
--to=will@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=dev.jain@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rppt@kernel.org \
--cc=ryan.roberts@arm.com \
--cc=shijie@os.amperecomputing.com \
--cc=yang@os.amperecomputing.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox