On 01/11/2022 09:51, Kevin Brodsky wrote:
On 25/10/2022 16:02, Beata Michalska wrote:
On Fri, Oct 21, 2022 at 02:33:51PM +0200, Kevin Brodsky wrote:
On 18/10/2022 22:20, Beata Michalska wrote:
> +/** > + * cheri_build_user_cap() - Create a userspace capability. > + * @addr: Requested capability address. > + * @len: Requested capability length. > + * @perms: Requested capability permissions. > + * > + * Returns a new capability derived from the root userspace > capability. Its > + * address and permissions are set according to @addr and @perms > respectively. > + * Its bounds are set exactly with @addr as base address and @len > as length. > + * > + * The caller is responsible to ensure that: > + * 1. @addr is a valid userspace address. > + * 2. The (@addr, @len) tuple can be represented as capability > bounds. > + * 3. @perms are valid permissions for a userspace capability. > + * > + * If either 1. or 2. does not hold, the resulting capability will > be invalid. > + * If 3. does not hold, the returned capability will not have any > of the invalid > + * permissions. > + */ > +void * __capability > +cheri_build_user_cap(ptraddr_t addr, size_t len, cheri_perms_t > perms); This builds full capability data so may be cheri_build_user_cap_data( used below for access function) or cheri_build_user_data.
I see your point. It all boils down to sealing in fact, as everything else is specified to the function as parameters. For cheri_check_cap_data_access() I thought it was important to say "data" as you cannot use this function to check a function pointer - it would be RB-sealed and therefore fail the sealing test.
I was less sure it made sense to say "data" for this function as it can be used to construct capabilities with any permissions, including Executable, but arguably it's also true that you can check for Executable using cheri_check_cap_data_access(). We should probably say "data" in both or neither.
I guess this choice is related to another, which is what we would do if we had to handle function pointers (I don't expect we will, but it helps thinking about it). There are essentially two options: 1. have a new set of functions with "code" in their name, or 2. use the same functions and do the sealing/unsealing manually. 1. corresponds to saying "data" for the present functions, and 2. to not saying "data".
Maybe we should keep it simple and go for 2. but opinions welcome here.
I am not entirely convinced that having the distinction here 'data' vs 'code' would be very much useful: this function could create both, and it can be extended to cover sealing case needed - not sure why manual sealing though...
I suppose we could indeed add an argument later to ask for a sentry (RB-sealed capability on Morello).
(cheri_root_seal_cap_userspace is already/will be there ?) (on a side note, sealed capability does not automatically denote function pointers even though that is not the use case we will be supporting, I guess).
We are specifically talking about sentries here (RB object type on Morello). It is expected that all function pointers are sentries in purecap.
Right, though that does not work the other way round: sentry (RB type sealed capability) is just one of the flavours of sealed capabilities. And even though for the cheri_check_cap_data_access, sealed capability is a no-go, as it is undereferenceable and we are checking for access permissions explicitly here,
That's indeed what I meant regarding sentries.
it is not so much obvious for building a capability: I guess we should be able to create sealed data capability ? (cannot think of a use case now though ... )
Possibly one day, though there is no such case in PCuABI as things stand. In any case I certain don't think that cheri.h as introduced by this patch is complete, it's simply a starting point with functionalities that will definitely be needed :)
On the other hand chari_check_cap_data_access could potentially check whether the cap has Executable permission set if selaed and consider that as valid ?
I would really like this to be explicit, but like the build function it could be a new argument.
I do understand the drive for being 'explicit' here, especially the longer I let it sink in.
So to summarise my thinking after your feedback (thanks for that!):
- cheri_build_user_cap() remains unchanged.
- cheri_check_cap_data_access() can be simply cheri_check_cap(), the
meaning of "check" being inferred from the arguments passed to the function.
No comments other than this seems reasonable given the above thread.
"data" is gone, if and when we want to handle things like function pointers we can add a new argument or create special functions.
Does that sound fair?
It does indeed.
Small note: compiler's CHERI intrinsics use cheri_cap_<action> notation, so we could go for that as well (cheri_cap_check) ? Just an idea, do not mind either.
That's a very valid point. However I'm rather unsure about "cheri_cap_build_user()", because "user" is about "cap" and not "build". Also note that "cap" is only present in "cheri_cap_build()", other functions are typically called "cheri_<attr>_{get,set}".
This also makes me wonder if using the same "cheri_" prefix as cheriintrin.h might not get confusing. On the other hand, I'm thinking of <linux/cheri.h> as the "new <cheriintrin.h>" for general use in the kernel, so cheriintrin.h could be seen as just an implementation detail for part of the "cheri_*" interface.
I'm on the fence on both of these angles, right now I would keep the names I proposed above unchanged but more opinions are welcome!
Agreed that cheri_build_user_cap() is clearer.
Thanks Zach
Kevin
BR. B
Kevin
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org
linux-morello mailing list -- linux-morello@op-lists.linaro.org To unsubscribe send an email to linux-morello-leave@op-lists.linaro.org