Re: [bug report] bad error return in walk_hugetlb_range()

[David Hildenbrand <david@redhat.com>]

On 04.10.25 08:22, Dan Carpenter wrote:
> This is really old code.  I think it's a bug in hugetlb.
> 
> 	drivers/gpu/drm/xe/xe_gt_pagefault.c:353 pf_queue_work_func()
> 	warn: passing positive error code 's32min-(-12),(-10)-(-1),1' to 'ERR_PTR'
> 
> mm/pagewalk.c
>     319  static int walk_hugetlb_range(unsigned long addr, unsigned long end,
>     320                                struct mm_walk *walk)
>     321  {
>     322          struct vm_area_struct *vma = walk->vma;
>     323          struct hstate *h = hstate_vma(vma);
>     324          unsigned long next;
>     325          unsigned long hmask = huge_page_mask(h);
>     326          unsigned long sz = huge_page_size(h);
>     327          pte_t *pte;
>     328          const struct mm_walk_ops *ops = walk->ops;
>     329          int err = 0;
>     330
>     331          hugetlb_vma_lock_read(vma);
>     332          do {
>     333                  next = hugetlb_entry_end(h, addr, end);
>     334                  pte = hugetlb_walk(vma, addr & hmask, sz);
>     335                  if (pte)
>     336                          err = ops->hugetlb_entry(pte, hmask, addr, next, walk);
> 
> The ->hugetlb_entry() is implemented by two functions which return
> true/false instead of error codes.  Smatch thinks this 1 value gets
> propagated back to pf_queue_work_func() and results an an Oops.
> 
> The two problem functions are hwpoison_hugetlb_range() and
> pagemap_hugetlb_range() which returns PM_END_OF_BUFFER from
> add_to_pagemap().

hwpoison_hugetlb_range() seems to behave just like hwpoison_pte_range(), 
returning "1" if check_hwpoisoned_entry() returned "1" -- if we found 
the entry with the problematic PFN and can just abort.

Staring at kill_accessing_process() that ends up calling these 
walk-functions, that seems to be correct. The value is converted to 
0/-EHWPOISON, all good.


pagemap_hugetlb_range() can indeed return either 0 or PM_END_OF_BUFFER 
obtained from add_to_pagemap().

But that's the same behavior as pagemap_pte_hole()/pagemap_pmd_range(), 
so that's nothing hugetlb-specific.

pagemap_read() does the
	ret = walk_page_range(mm, start_vaddr, end, &pagemap_ops, &pm);

After the loop, it does

if (!ret || ret == PM_END_OF_BUFFER)
	ret = copied;

So I don't immediately seeing anything wrong with that?

-- 
Cheers

David / dhildenb