Andrea Arcangeli wrote:
>
> ...
> Well, I think your earlier suggestion to bale out with an error if an
> invalid page is found sounds like the cleaner fix (possibly in function
> of yet another bitflag, so if somebody wants to get the nearby pages
> regardless of an invalid pages somewhere, it can).
>
I find it rather hard to decide about this. get_user_pages()
leaves null page pointers in the page[] array for invalid
pages, and that's a reasonable API, as long as all callers
are actually aware of it....
In the O_DIRECT case, the kernel does not crash, because
brw_kiovec() does:
map = iobuf->maplist[pageind];
if (!map) {
err = -EFAULT;
goto finished;
}
However, I think it _would_ crash if the first entry in the maplist[]
was non-null, and the second is null, because that would cause
generic_file_direct_IO() to call mark_dirty_kiobuf(), and
mark_dirty_kiobuf() forgets to check for NULL page *'s in the maplist[].
Given the difficulty of testing all this, and the dubious benefit
in allowing a holey maplist[], I'm inclined to just disallow it
in 2.4. What do you think?
--- linux-2.4.18-pre7/mm/memory.c Fri Dec 21 11:19:23 2001
+++ linux-akpm/mm/memory.c Mon Jan 28 16:26:47 2002
@@ -453,6 +453,7 @@ int get_user_pages(struct task_struct *t
vma = find_extend_vma(mm, start);
if ( !vma ||
+ (vma->vm_flags & VM_IO) ||
(!force &&
((write && (!(vma->vm_flags & VM_WRITE))) ||
(!write && (!(vma->vm_flags & VM_READ))) ) )) {
@@ -486,8 +487,9 @@ int get_user_pages(struct task_struct *t
/* FIXME: call the correct function,
* depending on the type of the found page
*/
- if (pages[i])
- page_cache_get(pages[i]);
+ if (!pages[i])
+ goto bad_page;
+ page_cache_get(pages[i]);
}
if (vmas)
vmas[i] = vma;
@@ -497,7 +499,19 @@ int get_user_pages(struct task_struct *t
} while(len && start < vma->vm_end);
spin_unlock(&mm->page_table_lock);
} while(len);
+out:
return i;
+
+ /*
+ * We found an invalid page in the VMA. Release all we have
+ * so far and fail.
+ */
+bad_page:
+ spin_unlock(&mm->page_table_lock);
+ while (i--)
+ page_cache_release(pages[i]);
+ i = -EFAULT;
+ goto out;
}
/*
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Thu Jan 31 2002 - 21:00:57 EST