Hi Tudor,
Thank you for the great review. I have missed quite a few things as you pointed them out, so well spotting!
Regarding all the changes being squashed into one single commit, this was actually intentional as I was not certain what kind of splitting is preferred in this case. The preference has been indeed voiced as I expected, so thank you for that. Will split them!
The only relation to the GUP is that the checks are made whenever the GUP functions are involved. Except for the only change in the GUP of course.
On 03/05/2023 18:12, Tudor Cretu wrote:
Related to this instance and the io_uring/kbuf.c, I think it would be better to add the check in io_pin_pages (a function defined by io_uring). Only that function calls pin_user_pages, the actual GUP function. Also, I don't think that to check for iov_len is sufficient if it doesn't represent an integer number of pages. What if the last page grabbed contains area outside of the capability bounds...
This is a rather complicated one. The point of view I am following here is that we are only checking that the address space for the user data is correctly delimited by the user capability. While in this case the kernel effectively grabs all the interested pages, the user space won't certainly gain access to them. Therefore, this is a matter of making sure that the kernel itself does not access parts of pages which are not covered by the provided capability bounds. If we care about this now, how could we tackle it then?
I can see this comes from __iov_iter_get_pages_alloc, but it still gives me a headache figuring out why it it like this...
I am there with you. I personally don't understand it either. I was indeed hoping that somebody would shed some light on it.
Best, Luca