Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
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


  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