Date: Thu, 12 Oct 2006 19:49:24 GMT From: Todd Miller <millert@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 107775 for review Message-ID: <200610121949.k9CJnO8P077381@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=107775 Change 107775 by millert@millert_macbook on 2006/10/12 19:48:49 Validate migscs input so we don't crash from corrupt data. Change how the message id to class (migscs) data is stored so we can free it properly when reloading. We now store the array of msgids associated with for each class in the classinfo struct itself. This means that to free the classinfo structs we just apply a function to each datum and free the datum when a reference count hits zero. Affected files ... .. //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/mach_av.c#4 edit Differences ... ==== //depot/projects/trustedbsd/sedarwin8/policies/sedarwin/sedarwin/ss/mach_av.c#4 (text+ko) ==== @@ -46,7 +46,8 @@ struct msgid_classinfo { - int baseid; + int nmsgids; + int *msgids; int nclasses; int classes[0]; /* actually larger */ }; @@ -55,6 +56,16 @@ lck_mtx_t *migscs_load_lock; +static int +msgid_destroy(void *key, void *datum, void *arg) +{ + struct msgid_classinfo *cinfo = datum; + + if (--cinfo->nmsgids == 0) + sebsd_free(cinfo, M_SEBSD); + return (0); +} + /* * Read the table mapping mach message ids to security classes. * The permissions in those classes are expected to be relative to the @@ -63,53 +74,72 @@ int sebsd_load_migscs(void *tdata, size_t tsize) { + struct msgid_classinfo *cinfo; struct hashtab *ht, *oht; - int error, *p, *ep; + int error, entries, i, msgid, nclasses, size; + int *p, *ep; ht = hashtab_create(msgid_hash, msgid_cmp, 31337); if (ht == NULL) return (-1); - printf("security class to subsystem table: %d classes\n", - tsize / sizeof(int)); - + error = 0; p = (int *)tdata; ep = (int *)((char *)tdata + tsize); - while (p < ep) { - int msgid = *p++; - int nclasses = *p++; - int size = *p++; - int i; - struct msgid_classinfo *c; + for (entries = 0; p < ep; entries++) { + if (p + 3 > ep) + goto bad; + msgid = *p++; + nclasses = *p++; + size = *p++; + if (msgid <= 0 || nclasses <= 0 || size <= 0 || + p + nclasses > ep) + goto bad; - c = sebsd_malloc(sizeof(int) * nclasses + sizeof(*c), M_SEBSD, - M_WAITOK); - c->baseid = msgid; - c->nclasses = nclasses; + /* + * We store the key in the data object it references + * so we can free things up with a map operation later. + */ + cinfo = sebsd_malloc(sizeof(*cinfo) + + (nclasses + size) * sizeof(int), M_SEBSD, M_WAITOK); + cinfo->nclasses = nclasses; for (i = 0; i < nclasses; i++) - c->classes[i] = *p++; - for (i = msgid; i < msgid + size; i++) { - int *ip = sebsd_malloc(sizeof(int), M_SEBSD, M_WAITOK); - *ip = i; - error = hashtab_insert(ht, ip, c); - if (error) { - hashtab_destroy(ht); - return (-1); - } + cinfo->classes[i] = *p++; + + cinfo->nmsgids = size; + cinfo->msgids = &cinfo->classes[nclasses]; + for (i = 0; i < size; i++) { + cinfo->msgids[i] = msgid + i; + error = hashtab_insert(ht, &cinfo->msgids[i], cinfo); + if (error) + goto bad; } } + printf("security class to subsystem table: %d classes\n", entries); + /* * Swap the old message id to class mapping with the new one * and free the old. - * XXX - does this leak memory? */ + /* XXX - need rwlock for msgid2class */ lck_mtx_lock(migscs_load_lock); oht = msgid2class; msgid2class = ht; lck_mtx_unlock(migscs_load_lock); - hashtab_destroy(oht); + if (oht != NULL) { + hashtab_map(oht, msgid_destroy, NULL); + hashtab_destroy(oht); + } return (0); +bad: + if (error) + printf("%s: hashtab_insert() error %d\n", __func__, error); + else + printf("%s: migscs data corrupted, ignoring\n", __func__); + hashtab_map(ht, msgid_destroy, NULL); + hashtab_destroy(ht); + return (-1); } void @@ -118,7 +148,6 @@ size_t tsize; void *tdata; - /* XXX - should also lookup & store migscs_path */ if (sebsd_find_data("migscs_data", &tdata, &tsize) != 0 || sebsd_load_migscs(tdata, tsize) != 0) msgid2class = hashtab_create(msgid_hash, msgid_cmp, 3); @@ -137,11 +166,12 @@ * Instead, we probably should make a check against a * new permission to be added to mach_port for this purpose. */ + /* XXX - need rwlock for msgid2class */ mcl = hashtab_search(msgid2class, &msgid); if (mcl == NULL) return 0; - cl = (msgid - mcl->baseid) / (8 * sizeof(u32)); + cl = (msgid - mcl->msgids[0]) / (8 * sizeof(u32)); if (cl >= mcl->nclasses) { /* bad message */ if (selinux_enforcing) @@ -150,6 +180,6 @@ return (0); } - perms = (u32)1 << (msgid - mcl->baseid - (cl * 8 * sizeof(u32))); + perms = (u32)1 << (msgid - mcl->msgids[0] - (cl * 8 * sizeof(u32))); return avc_has_perm(subj, obj, mcl->classes[cl], perms, NULL); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200610121949.k9CJnO8P077381>