From: David van Moolenbroek Date: Mon, 28 Dec 2015 23:48:39 +0000 (+0000) Subject: Add sequence numbers to grant IDs X-Git-Url: http://zhaoyanbai.com/repos/?a=commitdiff_plain;h=refs%2Fchanges%2F72%2F3272%2F3;p=minix.git Add sequence numbers to grant IDs The memory grant identifier for safecopies now includes a sequence number in its upper bits, to prevent accidental reuse of a grant ID after revocation and subsequent reallocation. This should increase overall system robustness by a tiny amount, and possibly help catch bugs in system services early on. For now, the lower 20 bits of the grant ID are used as grant table slot index (thus allowing for up to a million grants per process), and the next 11 bits of the (signed 32-bit) grant ID are used to store the per-slot sequence number. As grant IDs are never exposed to userland, the split can be changed later on without breaking the userland ABI. Change-Id: Ic34be27ff2a45db0ea5db037a24eef9efcd9ca40 --- diff --git a/minix/include/minix/safecopies.h b/minix/include/minix/safecopies.h index 5fe730339..1276cc6bf 100644 --- a/minix/include/minix/safecopies.h +++ b/minix/include/minix/safecopies.h @@ -1,4 +1,3 @@ - #ifndef _MINIX_SAFECOPIES_H #define _MINIX_SAFECOPIES_H 1 @@ -33,7 +32,8 @@ typedef struct { char cp_reserved[8]; /* future use */ } cp_magic; } cp_u; - char cp_reserved[8]; /* future use */ + int cp_seq; /* sequence number */ + char cp_reserved[4]; /* future use */ } cp_grant_t; /* Vectored safecopy. */ @@ -52,6 +52,14 @@ struct vscp_vec { #define GRANT_INVALID ((cp_grant_id_t) -1) #define GRANT_VALID(g) ((g) > GRANT_INVALID) +/* Grant index and sequence number split/merge/limits. */ +#define GRANT_SHIFT 20 /* seq: upper 11 bits, idx: lower 20 */ +#define GRANT_MAX_SEQ (1 << (31 - GRANT_SHIFT)) +#define GRANT_MAX_IDX (1 << GRANT_SHIFT) +#define GRANT_ID(idx, seq) ((cp_grant_id_t)((seq << GRANT_SHIFT) | (idx))) +#define GRANT_SEQ(g) (((g) >> GRANT_SHIFT) & (GRANT_MAX_SEQ - 1)) +#define GRANT_IDX(g) ((g) & (GRANT_MAX_IDX - 1)) + /* Operations: any combination is ok. */ #define CPF_READ 0x000001 /* Granted process may read. */ #define CPF_WRITE 0x000002 /* Granted process may write. */ @@ -69,11 +77,10 @@ struct vscp_vec { /* Prototypes for functions in libsys. */ cp_grant_id_t cpf_grant_direct(endpoint_t, vir_bytes, size_t, int); cp_grant_id_t cpf_grant_indirect(endpoint_t, endpoint_t, cp_grant_id_t); -cp_grant_id_t cpf_grant_magic(endpoint_t, endpoint_t, vir_bytes, size_t, - int); +cp_grant_id_t cpf_grant_magic(endpoint_t, endpoint_t, vir_bytes, size_t, int); int cpf_revoke(cp_grant_id_t grant_id); -int cpf_lookup(cp_grant_id_t g, endpoint_t *ep, endpoint_t *ep2); +/* START OF DEPRECATED API */ int cpf_getgrants(cp_grant_id_t *grant_ids, int n); int cpf_setgrant_direct(cp_grant_id_t g, endpoint_t who, vir_bytes addr, size_t size, int access); @@ -82,6 +89,8 @@ int cpf_setgrant_indirect(cp_grant_id_t g, endpoint_t who_to, endpoint_t int cpf_setgrant_magic(cp_grant_id_t g, endpoint_t who_to, endpoint_t who_from, vir_bytes addr, size_t bytes, int access); int cpf_setgrant_disable(cp_grant_id_t grant_id); +/* END OF DEPRECATED API */ + void cpf_reload(void); /* Set a process' grant table location and size (in-kernel only). */ @@ -91,4 +100,3 @@ void cpf_reload(void); priv(rp)->s_grant_endpoint= (rp)->p_endpoint; #endif /* _MINIX_SAFECOPIES_H */ - diff --git a/minix/kernel/system/do_safecopy.c b/minix/kernel/system/do_safecopy.c index 626fb85fa..5284741a5 100644 --- a/minix/kernel/system/do_safecopy.c +++ b/minix/kernel/system/do_safecopy.c @@ -44,9 +44,10 @@ int verify_grant( u32_t *flags /* CPF_* */ ) { - static cp_grant_t g; - static int proc_nr; - static const struct proc *granter_proc; + cp_grant_t g; + int proc_nr; + const struct proc *granter_proc; + int grant_idx, grant_seq; int depth = 0; do { @@ -93,23 +94,26 @@ int verify_grant( return(EPERM); } - if(priv(granter_proc)->s_grant_entries <= grant) { + grant_idx = GRANT_IDX(grant); + grant_seq = GRANT_SEQ(grant); + + if(priv(granter_proc)->s_grant_entries <= grant_idx) { printf( "verify_grant: grant verify failed in ep %d " - "proc %d: grant %d out of range " + "proc %d: grant 0x%x (#%d) out of range " "for table size %d\n", - granter, proc_nr, grant, + granter, proc_nr, grant, grant_idx, priv(granter_proc)->s_grant_entries); return(EPERM); } - /* Copy the grant entry corresponding to this id to see what it - * looks like. If it fails, hide the fact that granter has - * (presumably) set an invalid grant table entry by returning - * EPERM, just like with an invalid grant id. + /* Copy the grant entry corresponding to this ID's index to see + * what it looks like. If it fails, hide the fact that granter + * has (presumably) set an invalid grant table entry by + * returning EPERM, just like with an invalid grant id. */ - if(data_copy(granter, - priv(granter_proc)->s_grant_table + sizeof(g)*grant, + if(data_copy(granter, priv(granter_proc)->s_grant_table + + sizeof(g) * grant_idx, KERNEL, (vir_bytes) &g, sizeof(g)) != OK) { printf( "verify_grant: grant verify: data_copy failed\n"); @@ -118,12 +122,17 @@ int verify_grant( if(flags) *flags = g.cp_flags; - /* Check validity. */ + /* Check validity: flags and sequence number. */ if((g.cp_flags & (CPF_USED | CPF_VALID)) != (CPF_USED | CPF_VALID)) { - printf( - "verify_grant: grant failed: invalid (%d flags 0x%lx)\n", - grant, g.cp_flags); + printf("verify_grant: grant failed: invalid flags " + "(0x%x, 0x%lx)\n", grant, g.cp_flags); + return EPERM; + } + + if (g.cp_seq != grant_seq) { + printf("verify_grant: grant failed: invalid sequence " + "(0x%x, %d vs %d)\n", grant, grant_seq, g.cp_seq); return EPERM; } diff --git a/minix/lib/libsys/safecopies.c b/minix/lib/libsys/safecopies.c index 04c6454f9..f5b33c223 100644 --- a/minix/lib/libsys/safecopies.c +++ b/minix/lib/libsys/safecopies.c @@ -23,7 +23,8 @@ } #define GID_CHECK(gid) { \ - if(!GRANT_VALID(gid) || (gid) < 0 || (gid) >= ngrants) {\ + if(!GRANT_VALID(gid) || GRANT_IDX(gid) >= ngrants || \ + GRANT_SEQ(gid) != grants[GRANT_IDX(gid)].cp_seq) { \ errno = EINVAL; \ return -1; \ } \ @@ -31,19 +32,12 @@ #define GID_CHECK_USED(gid) { \ GID_CHECK(gid); \ - if(!(grants[gid].cp_flags & CPF_USED)) { \ + if(!(grants[GRANT_IDX(gid)].cp_flags & CPF_USED)) { \ errno = EINVAL; \ return -1; \ } \ } -#define CLICK_ALIGNMENT_CHECK(addr, bytes) { \ - if(((vir_bytes)(addr) % CLICK_SIZE != 0) \ - || ((vir_bytes)(bytes) % CLICK_SIZE != 0)) { \ - return EINVAL; \ - } \ - } - #define NR_STATIC_GRANTS 3 static cp_grant_t static_grants[NR_STATIC_GRANTS]; static cp_grant_t *grants = NULL; @@ -54,8 +48,7 @@ cpf_grow(void) { /* Grow the grants table if possible. */ cp_grant_t *new_grants; - cp_grant_id_t g; - int new_size; + int g, new_size; if(!ngrants) { /* Use statically allocated grants the first time. */ @@ -63,7 +56,12 @@ cpf_grow(void) new_grants = static_grants; } else { + /* Double(ish) the size, up to the maximum number of slots. */ + if (ngrants >= GRANT_MAX_IDX) + return; new_size = (1+ngrants)*2; + if (new_size >= GRANT_MAX_IDX) + new_size = GRANT_MAX_IDX; assert(new_size > ngrants); /* Allocate a block of new size. */ @@ -76,9 +74,15 @@ cpf_grow(void) if(grants && ngrants > 0) memcpy(new_grants, grants, ngrants * sizeof(grants[0])); - /* Make sure new slots are marked unused (CPF_USED is clear). */ - for(g = ngrants; g < new_size; g++) + /* + * Make sure new slots are marked unused (CPF_USED is clear). + * Also start with a zero sequence number, for consistency; since the + * grant table is never shrunk, this introduces no issues by itself. + */ + for(g = ngrants; g < new_size; g++) { new_grants[g].cp_flags = 0; + new_grants[g].cp_seq = 0; + } /* Inform kernel about new size (and possibly new location). */ if((sys_setgrant(new_grants, new_size))) { @@ -92,14 +96,14 @@ cpf_grow(void) ngrants = new_size; } -static cp_grant_id_t +static int cpf_new_grantslot(void) { /* Find a new, free grant slot in the grant table, grow it if * necessary. If no free slot is found and the grow failed, * return -1. Otherwise, return grant slot number. */ - cp_grant_id_t g; + int g; /* Find free slot. */ for(g = 0; g < ngrants && (grants[g].cp_flags & CPF_USED); g++) @@ -121,7 +125,6 @@ cpf_new_grantslot(void) /* Basic sanity checks - if we get this far, g must be a valid, * free slot. */ - assert(GRANT_VALID(g)); assert(g >= 0); assert(g < ngrants); assert(!(grants[g].cp_flags & CPF_USED)); @@ -132,24 +135,22 @@ cpf_new_grantslot(void) cp_grant_id_t cpf_grant_direct(endpoint_t who_to, vir_bytes addr, size_t bytes, int access) { - cp_grant_id_t g; - int r; + int g; + ACCESS_CHECK(access); + /* Get new slot to put new grant in. */ if((g = cpf_new_grantslot()) < 0) - return(GRANT_INVALID); - - assert(GRANT_VALID(g)); - assert(g >= 0); - assert(g < ngrants); - assert(!(grants[g].cp_flags & CPF_USED)); + return -1; - if((r=cpf_setgrant_direct(g, who_to, addr, bytes, access)) < 0) { - cpf_revoke(g); - return(GRANT_INVALID); - } + /* Fill in new slot data. */ + grants[g].cp_u.cp_direct.cp_who_to = who_to; + grants[g].cp_u.cp_direct.cp_start = addr; + grants[g].cp_u.cp_direct.cp_len = bytes; + __insn_barrier(); + grants[g].cp_flags = access | CPF_DIRECT | CPF_USED | CPF_VALID; - return g; + return GRANT_ID(g, grants[g].cp_seq); } cp_grant_id_t @@ -158,26 +159,20 @@ cpf_grant_indirect(endpoint_t who_to, endpoint_t who_from, cp_grant_id_t gr) /* Grant process A access into process B. B has granted us access as grant * id 'gr'. */ - cp_grant_id_t g; - int r; + int g; /* Obtain new slot. */ if((g = cpf_new_grantslot()) < 0) return -1; - /* Basic sanity checks. */ - assert(GRANT_VALID(g)); - assert(g >= 0); - assert(g < ngrants); - assert(!(grants[g].cp_flags & CPF_USED)); - /* Fill in new slot data. */ - if((r=cpf_setgrant_indirect(g, who_to, who_from, gr)) < 0) { - cpf_revoke(g); - return GRANT_INVALID; - } + grants[g].cp_u.cp_indirect.cp_who_to = who_to; + grants[g].cp_u.cp_indirect.cp_who_from = who_from; + grants[g].cp_u.cp_indirect.cp_grant = gr; + __insn_barrier(); + grants[g].cp_flags = CPF_USED | CPF_INDIRECT | CPF_VALID; - return g; + return GRANT_ID(g, grants[g].cp_seq); } cp_grant_id_t @@ -185,8 +180,7 @@ cpf_grant_magic(endpoint_t who_to, endpoint_t who_from, vir_bytes addr, size_t bytes, int access) { /* Grant process A access into process B. Not everyone can do this. */ - cp_grant_id_t g; - int r; + int g; ACCESS_CHECK(access); @@ -194,52 +188,56 @@ cpf_grant_magic(endpoint_t who_to, endpoint_t who_from, if((g = cpf_new_grantslot()) < 0) return -1; - /* Basic sanity checks. */ - assert(GRANT_VALID(g)); - assert(g >= 0); - assert(g < ngrants); - assert(!(grants[g].cp_flags & CPF_USED)); - - if((r=cpf_setgrant_magic(g, who_to, who_from, addr, - bytes, access)) < 0) { - cpf_revoke(g); - return -1; - } - - return g; + /* Fill in new slot data. */ + grants[g].cp_u.cp_magic.cp_who_to = who_to; + grants[g].cp_u.cp_magic.cp_who_from = who_from; + grants[g].cp_u.cp_magic.cp_start = addr; + grants[g].cp_u.cp_magic.cp_len = bytes; + __insn_barrier(); + grants[g].cp_flags = CPF_USED | CPF_MAGIC | CPF_VALID | access; + + return GRANT_ID(g, grants[g].cp_seq); } int -cpf_revoke(cp_grant_id_t g) +cpf_revoke(cp_grant_id_t grant) { /* Revoke previously granted access, identified by grant id. */ - GID_CHECK_USED(g); + int g; + + GID_CHECK_USED(grant); + + g = GRANT_IDX(grant); /* Make grant invalid by setting flags to 0, clearing CPF_USED. * This invalidates the grant. */ grants[g].cp_flags = 0; + __insn_barrier(); - return 0; -} - -int -cpf_lookup(cp_grant_id_t g, endpoint_t *granter, endpoint_t *grantee) -{ - /* First check slot validity, and if it's in use currently. */ - GID_CHECK_USED(g); - - if(grants[g].cp_flags & CPF_DIRECT) { - if(granter) *granter = SELF; - if(grantee) *grantee = grants[g].cp_u.cp_direct.cp_who_to; - } else if(grants[g].cp_flags & CPF_MAGIC) { - if(granter) *granter = grants[g].cp_u.cp_magic.cp_who_from; - if(grantee) *grantee = grants[g].cp_u.cp_magic.cp_who_to; - } else return -1; + /* + * Increase the grant slot's sequence number now, rather than on + * allocation, because live update relies on the first allocated grant + * having a zero ID (SEF_STATE_TRANSFER_GID) and thus a zero sequence + * number. + */ + if (grants[g].cp_seq < GRANT_MAX_SEQ - 1) + grants[g].cp_seq++; + else + grants[g].cp_seq = 0; return 0; } +/* + * START OF DEPRECATED API + * + * The grant preallocation and (re)assignment API below imposes that grant IDs + * stay the same across reuse, thus disallowing that the grants' sequence + * numbers be updated as a part of reassignment. As a result, this API does + * not offer the same protection against accidental reuse of an old grant by a + * remote party as the regular API does, and is therefore deprecated. + */ int cpf_getgrants(cp_grant_id_t *grant_ids, int n) { @@ -249,6 +247,7 @@ cpf_getgrants(cp_grant_id_t *grant_ids, int n) if((grant_ids[i] = cpf_new_grantslot()) < 0) break; grants[grant_ids[i]].cp_flags = CPF_USED; + grants[grant_ids[i]].cp_seq = 0; } /* return however many grants were assigned. */ @@ -324,6 +323,9 @@ cp_grant_id_t gid; return 0; } +/* + * END OF DEPRECATED API + */ void cpf_reload(void) @@ -334,4 +336,3 @@ cpf_reload(void) if (grants) sys_setgrant(grants, ngrants); /* Do we need error checking? */ } -