On Sat, Apr 15, 2023 at 08:40:44PM +0900, Tetsuo Handa wrote:
> On 2023/04/15 20:27, Lorenzo Stoakes wrote:
> >>> @@ -5617,11 +5618,11 @@ int __access_remote_vm(struct mm_struct *mm, unsigned long addr, void *buf,
> >>> bytes = PAGE_SIZE-offset;
> >>>
> >>> maddr = kmap(page);
> >>> - if (write) {
> >>> + if (write && vma) {
> >>> copy_to_user_page(vma, page, addr,
> >>> maddr + offset, buf, bytes);
> >>> set_page_dirty_lock(page);
> >>> - } else {
> >>> + } else if (vma) {
> >>> copy_from_user_page(vma, page, addr,
> >>> buf, maddr + offset, bytes);
> >>> }
> >>
> >> not calling copy_{from,to}_user_page() if vma == NULL is not sufficient for
> >> propagating an error to caller.
> >>
> >
> > This is a product of wanting to avoid churn, again this condition is simply
> > impossible. Also as a pedantic side note - the loop explicitly indicates no
> > errors are propagated, so there is no need to do so.
>
> Since __access_remote_vm() implicitly indicates an error via "return buf - old_buf;",
> "buf += bytes;" must not be executed if copy_{to,from}_user_page(bytes) was not called.
>
>
Yes, indeed, perhaps I wasn't clear, ack that we shouldn't increment buf if
!vma, I already fixed this in the respin and additionally have added a
warning here and in every instance of unexpected !vma.
Will spam everybody with v3 once the allmodconfig build is complete... :)
