On Thu, Jun 30, 2022 at 10:41:23AM +0200, Linus Walleij wrote:
> Functions that work on a pointer to virtual memory such as
> virt_to_pfn() and users of that function such as
> virt_to_page() are supposed to pass a pointer to virtual
> memory, ideally a (void *) or other pointer. However since
> many architectures implement virt_to_pfn() as a macro,
> this function becomes polymorphic and accepts both a
> (unsigned long) and a (void *).
I wonder if there is merit to convert x86 to use an inline after this
goes in to prevent this polymorphic mistake?
> If we instead implement a proper virt_to_pfn(void *addr)
> function the following happens (occurred on arch/arm):
>
> mm/gup.c: In function '__get_user_pages_locked':
> mm/gup.c:1599:49: warning: passing argument 1 of 'virt_to_pfn'
> makes pointer from integer without a cast [-Wint-conversion]
> pages[i] = virt_to_page(start);
>
> Fix this with an explicit cast.
>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: linux-mm@xxxxxxxxx
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> mm/gup.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/gup.c b/mm/gup.c
> index 551264407624..543c68da65f1 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -1672,7 +1672,7 @@ static long __get_user_pages_locked(struct mm_struct *mm, unsigned long start,
> goto finish_or_fault;
>
> if (pages) {
> - pages[i] = virt_to_page(start);
> + pages[i] = virt_to_page((void *)start);
That 'start' is actually a userspace addres so it technically is a
__user pointer, but the missing context here is that this is a NOMMU
special function, so I guess it is right as is?
Still, it is NOP to what it is now so:
Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
Jason
