Skip to contentSkip to conversation

mm/gup.c: Fix return value for __gup_longterm_locked()

bug report Dan Carpenter 2 participants 2 messages Aug 25, 2022 – Aug 27, 2022 Memory management

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw

Hi Dan,

Quoted 31 lines · 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)

Quoted 2 lines

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

Quoted 13 lines
    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