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.
