Re: [PATCH v2] riscv: mm: add null check for find_vm_area in __set_memory

["Lorenzo Stoakes (Oracle)" <ljs@kernel.org>]

(-cc old email address +cc new.)

On Mon, Mar 16, 2026 at 04:16:39PM +0100, Osama Abdelkader wrote:
> find_vm_area() can return NULL. Add a null check to avoid potential
> null pointer dereference, matching the pattern used by other arches.
>
> Fixes: 311cd2f6e253 ("riscv: Fix set_memory_XX() and set_direct_map_XX() by splitting huge linear mappings")
> Cc: stable@vger.kernel.org
> Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
> ---
> v2:
> - Add Cc: stable@vger.kernel.org
> - Add Fixes: tag

This isn't a bug AFAICT, and we'd only really cc: stable add fixes if it was
identifiable as one, as Andrew mentions.

> - mention __set_memory in the commit message
> ---
>  arch/riscv/mm/pageattr.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/mm/pageattr.c b/arch/riscv/mm/pageattr.c
> index 3f76db3d2769..46a999c86b26 100644
> --- a/arch/riscv/mm/pageattr.c
> +++ b/arch/riscv/mm/pageattr.c
> @@ -289,6 +289,10 @@ static int __set_memory(unsigned long addr, int numpages, pgprot_t set_mask,
>  		int i, page_start;
>
>  		area = find_vm_area((void *)start);
> +		if (!area) {
> +			ret = -EINVAL;
> +			goto unlock;
> +		}

This call is gated on is_vmalloc_or_module_addr() so how would we fail to find
an area here?  (modules are also vmalloc()'d)

All set_memory_*() callers will be referencing genuine live data also, so I
don't think this is an issue?

Other arches do a NULL check, but they are not explicitly checking
is_vmalloc_or_module_addr() before doing the check, they seem to be using this
== NULL to imply the memory is something else.

So I think this patch is not correct, except for cases of some underlying bug,
but a bug SURELY would have triggered by now?

So yeah I don't think we should take this patch, as it implies a case that
simply cannot happen.

If it does happen and we get a bug report, it'll be very obvious where it
happened and why.

>  		page_start = (start - (unsigned long)area->addr) >> PAGE_SHIFT;
>
>  		for (i = page_start; i < page_start + numpages; ++i) {
> --
> 2.43.0
>

Thanks, Lorenzo