On 9/18/25 8:32 AM, Catalin Marinas wrote:

On Thu, Sep 18, 2025 at 08:05:55AM -0700, Yang Shi wrote:

quoted

On 9/18/25 5:48 AM, Catalin Marinas wrote:

quoted

On Wed, Sep 17, 2025 at 12:02:11PM -0700, Yang Shi wrote:

quoted

+	page = execmem_alloc(EXECMEM_KPROBES, PAGE_SIZE);
+	if (!page)
+		return NULL;
+	set_memory_rox((unsigned long)page, 1);

It's unfortunate that we change the attributes of the ROX vmap first to
RO, then to back to ROX so that we get the linear map changed. Maybe
factor out some of the code in change_memory_common() to only change the
linear map.

I want to make sure I understand you correctly, you meant set_memory_rox()
should do:

change linear map to RO (call a new helper, for example,
set_direct_map_ro())
change vmap to ROX (call change_memory_common())

set_memory_rox() is correct. What I meant is that in alloc_insn_page(),
execmem_alloc() already returns RX memory. Calling set_memory_rox() does
indeed change the linear map to RO but it also changes the vmap memory
to RO and then to RX. There's no need for the alloc_insn_page() to do
this but we shouldn't change set_memory_rox() for this, the latter is
correct. I was thinking of alloc_insn_page() calling a new function that
only changes the linear map.

Aha, I see. If we have the new helper, it also allows us to refactor 
set_memory_rox() to what I said.

quoted

And I think we should have the cleanup patch separate from this bug fix
patch because the bug fix patch should be applied to -stable release too.
Keeping it simpler makes the backport easier.

Yes, for now you can leave it as is, that's not a critical path.

Sure.

quoted

Shall I squash the cleanup patch into patch #1?

No, I'd leave it as a separate fix, especially if we want to backport
it.

I meant the potential cleanup patch. Anyway it can be handled in a 
separate patch at anytime.

Anyway, for now, with the nitpick on the address variable name:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Thank you. Ryan also suggested separate the fix from this series. I will 
fix the variable name nit then post it separately instead of posting a 
new series.

Yang