On 01/11/2022 19:16, Zachary Leaf wrote:
On 01/11/2022 15:11, Teo Couprie Diaz wrote:
On 01/11/2022 10:03, Kevin Brodsky wrote:
On 25/10/2022 16:32, Beata Michalska wrote:
On Fri, Oct 21, 2022 at 02:34:48PM +0200, Kevin Brodsky wrote:
On 18/10/2022 22:16, Beata Michalska wrote:
>> Minor: How about make_user_ptr_with_capability()? privileged may >> confuse >> with privilege mode or something else. Although I don't have strong >> opinion here. > I agree, "privileged" is a very overloaded term. What I am trying > to convey > is that the returned user pointer is to be used by the kernel > itself only, > because we do not control how the bounds are extended for > representability > purposes. This is fine inside the kernel as we generally cannot do > better > (and have enough privilege to access any part of the address > space), but it > is not if userspace gets hold of this capability, as it could give > access to > arbitrary objects in addition to the one we intended. > Additionally, we > provide all possible Load/Store capability permissions as > requested, which > is again fine when the kernel controls the access but not > necessarily if > userspace does (e.g. we may want to provide Load but not LoadCap). > > It is in this sense that the user pointer is "privileged" (because > only the > kernel should use it), but I agree that word is misleading. Maybe > this is > too subtle to convey in the naming. Since there are very few > places in which > the kernel creates user pointers for userspace (I think just > binfmt_elf + > mmap & co, where the CHERI API should be used directly instead of > this one), > the risks of accidental use of this function may be low enough > that we can > keep the name simple. Definitely looking for opinions on this! > Jumping here on this one to hopefully keep the discussion in a single thread...
So what we are looking at is capability with potentially elevated permissions and extended boundaries under sole kernel control... How about create_unrestricted_user_ptr instead ? or create_relaxed_user_ptr ?
"unrestricted" sounds deceiving as bounds and permissions are restricted, though not precisely. "relaxed" sounds a little bit strange for a pointer :D
That said these are inspiring, how about "loose"? "make_loose_user_ptr()"? It is hard to understand what that would refer to outside of the CHERI context, but I'm not sure we can do much better.
That still leaves it bit vague (as per your comment re: restricted bounds and permissions). This is tricky to get it right. 'managed' pop into my mind at some point: as the capability created will be handled internally by the kernel only, but it does not sound entirely appealing either. That said, I think I am out of 'reasonable' ideas at this point, sadly.
I feel that "managed" is not much less vague than "loose", which I don't really like either right now...
In the end we should probably either have no annotation or one pointing to what we actually mean, so it could be something like "inkernel", "make_inkernel_user_ptr()"? It looks rather weird, but maybe that's the point, it might get the user or reader to check the function documentation :)
As you said above it is a subtle thing you want the name to convey, and I feel that while weird "make_inkernel_user_ptr()" might fit. At least it makes sense with the details above, and the name hints that it should be "in the kernel".
A bit of an in-between proposition compared to the previous ones : maybe "unsafe" could work ? From your explanation it is indeed unsafe to share to user space, even though it _is_ restricted. It can be loose, but loose itself is unsafe as well. It also hints that you should be wary while using this, but maybe too strongly once again ?
Lots of options have already been discussed so it's hard to think of others, but "inkernel" does feel like it's the closest approximation of what you want to convey. My "unsafe" idea is probably discouraging using the function too much.
How about make_kernel_only_user_ptr()? The kernel only part seems to be the crux, and using the function would serve as a reminder of that.
Discussed this offline. Qualifying this function is particularly tricky as the meaning is so subtle (and new in a kernel context). In the end Téo's suggestion to use "unsafe" seems to be the most palatable as it does give a warning without being specific about what this warning is, inviting the user to read the function description. Therefore I'd go for make_user_ptr_unsafe().
Thanks everyone for the suggestions, that was very helpful!
Long shot, but I guess there is not many spare or unused bits around - otherwise we could have a kernel only flag + uaccess copy_to_user routines would fail if it was set to avoid leaking privileged caps.
This is quite an interesting idea, I hadn't considered that :) We only use one User bit at the moment so reserving another for internal kernel usage is plausible. The main trouble here is that explicit checking would be needed, adding a lot of branches to copy_to_user_with_ptr(). It might be acceptable if its performance is not too critical, but this is hard to say at this point. Worth keeping that idea in mind though!
Kevin