[bug report] mm/gup.c: Fix return value for __gup_longterm_locked()

[bug report] mm/gup.c: Fix return value for __gup_longterm_locked()

[Dan Carpenter]

Hello Shigeru Yoshida,

The patch 35f9c4607f9e: "mm/gup.c: Fix return value for
__gup_longterm_locked()" from Aug 22, 2022, leads to the following
Smatch static checker warning:

	mm/gup.c:2089 __gup_longterm_locked()
	error: uninitialized symbol 'rc'.

mm/gup.c
    2064 static long __gup_longterm_locked(struct mm_struct *mm,
    2065                                   unsigned long start,
    2066                                   unsigned long nr_pages,
    2067                                   struct page **pages,
    2068                                   struct vm_area_struct **vmas,
    2069                                   unsigned int gup_flags)
    2070 {
    2071         unsigned int flags;
    2072         long rc, nr_pinned_pages;
    2073 
    2074         if (!(gup_flags & FOLL_LONGTERM))
    2075                 return __get_user_pages_locked(mm, start, nr_pages, pages, vmas,
    2076                                                NULL, gup_flags);
    2077         flags = memalloc_pin_save();
    2078         do {
    2079                 nr_pinned_pages = __get_user_pages_locked(mm, start, nr_pages,
    2080                                                           pages, vmas, NULL,
    2081                                                           gup_flags);
    2082                 if (nr_pinned_pages <= 0)


This doesn't work if if fails on the first iteration.  But also I'm
surprised that we are return -EAGAIN on subsequent iterations...

    2083                         break;
    2084                 rc = check_and_migrate_movable_pages(nr_pinned_pages, pages,
    2085                                                      gup_flags);
    2086         } while (rc == -EAGAIN);
    2087         memalloc_pin_restore(flags);
    2088 
--> 2089         return rc ? rc : nr_pinned_pages;
    2090 }

regards,
dan carpenter

Re: [bug report] mm/gup.c: Fix return value for __gup_longterm_locked()

[Shigeru Yoshida]

Hi Dan,

On Thu, 25 Aug 2022 10:49:27 +0300, Dan Carpenter wrote:
> Hello Shigeru Yoshida,
> 
> The patch 35f9c4607f9e: "mm/gup.c: Fix return value for
> __gup_longterm_locked()" from Aug 22, 2022, leads to the following
> Smatch static checker warning:
> 
> 	mm/gup.c:2089 __gup_longterm_locked()
> 	error: uninitialized symbol 'rc'.
> 
> mm/gup.c
>     2064 static long __gup_longterm_locked(struct mm_struct *mm,
>     2065                                   unsigned long start,
>     2066                                   unsigned long nr_pages,
>     2067                                   struct page **pages,
>     2068                                   struct vm_area_struct **vmas,
>     2069                                   unsigned int gup_flags)
>     2070 {
>     2071         unsigned int flags;
>     2072         long rc, nr_pinned_pages;
>     2073 
>     2074         if (!(gup_flags & FOLL_LONGTERM))
>     2075                 return __get_user_pages_locked(mm, start, nr_pages, pages, vmas,
>     2076                                                NULL, gup_flags);
>     2077         flags = memalloc_pin_save();
>     2078         do {
>     2079                 nr_pinned_pages = __get_user_pages_locked(mm, start, nr_pages,
>     2080                                                           pages, vmas, NULL,
>     2081                                                           gup_flags);
>     2082                 if (nr_pinned_pages <= 0)
> 
> 
> This doesn't work if if fails on the first iteration.  But also I'm
> surprised that we are return -EAGAIN on subsequent iterations...

Sorry for late response and thanks for your bug report.  Yes, that's
my fault.  I'll prepare the patch.

Thanks,
Shigeru

> 
>     2083                         break;
>     2084                 rc = check_and_migrate_movable_pages(nr_pinned_pages, pages,
>     2085                                                      gup_flags);
>     2086         } while (rc == -EAGAIN);
>     2087         memalloc_pin_restore(flags);
>     2088 
> --> 2089         return rc ? rc : nr_pinned_pages;
>     2090 }
> 
> regards,
> dan carpenter
>