From 852e22950dc47e774bb602b16f55fed42afac5fb Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:36 -0500 Subject: cifs: use the NUM_AUTHS and NUM_SUBAUTHS constants in cifsacl code ...instead of hardcoding in '5' and '6' all over the place. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 5c902c7ce524..80e0d66a403d 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -60,8 +60,8 @@ struct cifs_ntsd { struct cifs_sid { __u8 revision; /* revision level */ __u8 num_subauth; - __u8 authority[6]; - __le32 sub_auth[5]; /* sub_auth[num_subauth] */ + __u8 authority[NUM_AUTHS]; + __le32 sub_auth[NUM_SUBAUTHS]; /* sub_auth[num_subauth] */ } __attribute__((packed)); struct cifs_acl { -- cgit v1.2.3 From 436bb435fcbe2d52678ec7e2abc45fd1938601ce Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:36 -0500 Subject: cifs: make compare_sids static ..nothing outside of cifsacl.c calls it. Also fix the incorrect comment on the function. It returns 0 when they match. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 98 ++++++++++++++++++++++++++++--------------------------- fs/cifs/cifsacl.h | 2 -- 2 files changed, 50 insertions(+), 50 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 18437c5561fe..5a312eb45a92 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -224,6 +224,56 @@ sid_to_str(struct cifs_sid *sidptr, char *sidstr) } } +/* + * if the two SIDs (roughly equivalent to a UUID for a user or group) are + * the same returns zero, if they do not match returns non-zero. + */ +static int +compare_sids(const struct cifs_sid *ctsid, const struct cifs_sid *cwsid) +{ + int i; + int num_subauth, num_sat, num_saw; + + if ((!ctsid) || (!cwsid)) + return 1; + + /* compare the revision */ + if (ctsid->revision != cwsid->revision) { + if (ctsid->revision > cwsid->revision) + return 1; + else + return -1; + } + + /* compare all of the six auth values */ + for (i = 0; i < NUM_AUTHS; ++i) { + if (ctsid->authority[i] != cwsid->authority[i]) { + if (ctsid->authority[i] > cwsid->authority[i]) + return 1; + else + return -1; + } + } + + /* compare all of the subauth values if any */ + num_sat = ctsid->num_subauth; + num_saw = cwsid->num_subauth; + num_subauth = num_sat < num_saw ? num_sat : num_saw; + if (num_subauth) { + for (i = 0; i < num_subauth; ++i) { + if (ctsid->sub_auth[i] != cwsid->sub_auth[i]) { + if (le32_to_cpu(ctsid->sub_auth[i]) > + le32_to_cpu(cwsid->sub_auth[i])) + return 1; + else + return -1; + } + } + } + + return 0; /* sids compare/match */ +} + static void cifs_copy_sid(struct cifs_sid *dst, const struct cifs_sid *src) { @@ -630,54 +680,6 @@ cifs_destroy_idmaptrees(void) spin_unlock(&gidsidlock); } -/* if the two SIDs (roughly equivalent to a UUID for a user or group) are - the same returns 1, if they do not match returns 0 */ -int compare_sids(const struct cifs_sid *ctsid, const struct cifs_sid *cwsid) -{ - int i; - int num_subauth, num_sat, num_saw; - - if ((!ctsid) || (!cwsid)) - return 1; - - /* compare the revision */ - if (ctsid->revision != cwsid->revision) { - if (ctsid->revision > cwsid->revision) - return 1; - else - return -1; - } - - /* compare all of the six auth values */ - for (i = 0; i < NUM_AUTHS; ++i) { - if (ctsid->authority[i] != cwsid->authority[i]) { - if (ctsid->authority[i] > cwsid->authority[i]) - return 1; - else - return -1; - } - } - - /* compare all of the subauth values if any */ - num_sat = ctsid->num_subauth; - num_saw = cwsid->num_subauth; - num_subauth = num_sat < num_saw ? num_sat : num_saw; - if (num_subauth) { - for (i = 0; i < num_subauth; ++i) { - if (ctsid->sub_auth[i] != cwsid->sub_auth[i]) { - if (le32_to_cpu(ctsid->sub_auth[i]) > - le32_to_cpu(cwsid->sub_auth[i])) - return 1; - else - return -1; - } - } - } - - return 0; /* sids compare/match */ -} - - /* copy ntsd, owner sid, and group sid from a security descriptor to another */ static void copy_sec_desc(const struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, __u32 sidsoffset) diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 80e0d66a403d..18c7521273a7 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -98,6 +98,4 @@ extern struct key_type cifs_idmap_key_type; extern const struct cred *root_cred; #endif /* KERNEL */ -extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *); - #endif /* _CIFSACL_H */ -- cgit v1.2.3 From 36f87ee70f754d04e55518853e6fb30ed4732dda Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:37 -0500 Subject: cifs: make cifs_copy_sid handle a source sid with variable size subauth arrays ...and lift the restriction in id_to_sid upcall that the size must be at least as big as a full cifs_sid. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 10 ++++++++-- fs/cifs/cifsacl.h | 3 +++ 2 files changed, 11 insertions(+), 2 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 5a312eb45a92..141a944c9dfd 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -277,8 +277,14 @@ compare_sids(const struct cifs_sid *ctsid, const struct cifs_sid *cwsid) static void cifs_copy_sid(struct cifs_sid *dst, const struct cifs_sid *src) { - memcpy(dst, src, sizeof(*dst)); + int i; + + dst->revision = src->revision; dst->num_subauth = min_t(u8, src->num_subauth, NUM_SUBAUTHS); + for (i = 0; i < NUM_AUTHS; ++i) + dst->authority[i] = src->authority[i]; + for (i = 0; i < dst->num_subauth; ++i) + dst->sub_auth[i] = src->sub_auth[i]; } static void @@ -427,7 +433,7 @@ id_to_sid(unsigned long cid, uint sidtype, struct cifs_sid *ssid) if (IS_ERR(sidkey)) { rc = -EINVAL; cFYI(1, "%s: Can't map and id to a SID", __func__); - } else if (sidkey->datalen < sizeof(struct cifs_sid)) { + } else if (sidkey->datalen < CIFS_SID_BASE_SIZE) { rc = -EIO; cFYI(1, "%s: Downcall contained malformed key " "(datalen=%hu)", __func__, sidkey->datalen); diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 18c7521273a7..7e52f19f996f 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -64,6 +64,9 @@ struct cifs_sid { __le32 sub_auth[NUM_SUBAUTHS]; /* sub_auth[num_subauth] */ } __attribute__((packed)); +/* size of a struct cifs_sid, sans sub_auth array */ +#define CIFS_SID_BASE_SIZE (1 + 1 + NUM_AUTHS) + struct cifs_acl { __le16 revision; /* revision level */ __le16 size; -- cgit v1.2.3 From 30c9d6cca526243abe6c08eb6fa03db9d2b1a630 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:37 -0500 Subject: cifs: redefine NUM_SUBAUTH constant from 5 to 15 According to several places on the Internet and the samba winbind code, this is hard limited to 15 in windows, not 5. This does balloon out the allocation of each by 40 bytes, but I don't see any alternative. Also, rename it to SID_MAX_SUB_AUTHORITIES to match the alleged name of this constant in the windows header files Finally, rename SIDLEN to SID_STRING_MAX, fix the value to reflect the change to SID_MAX_SUB_AUTHORITIES and document how it was determined. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 6 +++--- fs/cifs/cifsacl.h | 19 ++++++++++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 141a944c9dfd..dd8d3df74298 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -280,7 +280,7 @@ cifs_copy_sid(struct cifs_sid *dst, const struct cifs_sid *src) int i; dst->revision = src->revision; - dst->num_subauth = min_t(u8, src->num_subauth, NUM_SUBAUTHS); + dst->num_subauth = min_t(u8, src->num_subauth, SID_MAX_SUB_AUTHORITIES); for (i = 0; i < NUM_AUTHS; ++i) dst->authority[i] = src->authority[i]; for (i = 0; i < dst->num_subauth; ++i) @@ -383,7 +383,7 @@ id_to_sid(unsigned long cid, uint sidtype, struct cifs_sid *ssid) if (!npsidid) return -ENOMEM; - npsidid->sidstr = kmalloc(SIDLEN, GFP_KERNEL); + npsidid->sidstr = kmalloc(SID_STRING_MAX, GFP_KERNEL); if (!npsidid->sidstr) { kfree(npsidid); return -ENOMEM; @@ -500,7 +500,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb, struct cifs_sid *psid, if (!npsidid) return -ENOMEM; - npsidid->sidstr = kmalloc(SIDLEN, GFP_KERNEL); + npsidid->sidstr = kmalloc(SID_STRING_MAX, GFP_KERNEL); if (!npsidid->sidstr) { kfree(npsidid); return -ENOMEM; diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 7e52f19f996f..8b980cd445c0 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -24,7 +24,7 @@ #define NUM_AUTHS 6 /* number of authority fields */ -#define NUM_SUBAUTHS 5 /* number of sub authority fields */ +#define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */ #define NUM_WK_SIDS 7 /* number of well known sids */ #define SIDNAMELENGTH 20 /* long enough for the ones we care about */ #define DEFSECDESCLEN 192 /* sec desc len contaiting a dacl with three aces */ @@ -41,7 +41,20 @@ #define SIDOWNER 1 #define SIDGROUP 2 -#define SIDLEN 150 /* S- 1 revision- 6 authorities- max 5 sub authorities */ + +/* + * Maximum size of a string representation of a SID: + * + * The fields are unsigned values in decimal. So: + * + * u8: max 3 bytes in decimal + * u32: max 10 bytes in decimal + * + * "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes + * per number + 1 for '-') + 11 bytes for each subauthority field (10 bytes + * per number + 1 for '-') + NULL terminator. + */ +#define SID_STRING_MAX (195) #define SID_ID_MAPPED 0 #define SID_ID_PENDING 1 @@ -61,7 +74,7 @@ struct cifs_sid { __u8 revision; /* revision level */ __u8 num_subauth; __u8 authority[NUM_AUTHS]; - __le32 sub_auth[NUM_SUBAUTHS]; /* sub_auth[num_subauth] */ + __le32 sub_auth[SID_MAX_SUB_AUTHORITIES]; /* sub_auth[num_subauth] */ } __attribute__((packed)); /* size of a struct cifs_sid, sans sub_auth array */ -- cgit v1.2.3 From b1a6dc21d1a731fdb71fcd683ef856c6af0b3f23 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Sun, 25 Nov 2012 08:00:38 -0500 Subject: cifs: remove uneeded __KERNEL__ block from cifsacl.h ...and make those symbols static in cifsacl.c. Nothing outside of that file refers to them. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 4 ++-- fs/cifs/cifsacl.h | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 9adcdb5a1001..42b3fe981a0a 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -42,7 +42,7 @@ static const struct cifs_sid sid_authusers = { /* group users */ static const struct cifs_sid sid_user = {1, 2 , {0, 0, 0, 0, 0, 5}, {} }; -const struct cred *root_cred; +static const struct cred *root_cred; static void shrink_idmap_tree(struct rb_root *root, int nr_to_scan, int *nr_rem, @@ -187,7 +187,7 @@ cifs_idmap_key_destroy(struct key *key) kfree(key->payload.data); } -struct key_type cifs_idmap_key_type = { +static struct key_type cifs_idmap_key_type = { .name = "cifs.idmap", .instantiate = cifs_idmap_key_instantiate, .destroy = cifs_idmap_key_destroy, diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 8b980cd445c0..249c94f39635 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -109,9 +109,4 @@ struct cifs_sid_id { struct cifs_sid sid; }; -#ifdef __KERNEL__ -extern struct key_type cifs_idmap_key_type; -extern const struct cred *root_cred; -#endif /* KERNEL */ - #endif /* _CIFSACL_H */ -- cgit v1.2.3 From faa65f07d21e7d37190c91fdcf9f940d733ae3cc Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 3 Dec 2012 06:05:29 -0500 Subject: cifs: simplify id_to_sid and sid_to_id mapping code The cifs.idmap handling code currently causes the kernel to cache the data from userspace twice. It first looks in a rbtree to see if there is a matching entry for the given id. If there isn't then it calls request_key which then checks its cache and then calls out to userland if it doesn't have one. If the userland program establishes a mapping and downcalls with that info, it then gets cached in the keyring and in this rbtree. Aside from the double memory usage and the performance penalty in doing all of these extra copies, there are some nasty bugs in here too. The code declares four rbtrees and spinlocks to protect them, but only seems to use two of them. The upshot is that the same tree is used to hold (eg) uid:sid and sid:uid mappings. The comparitors aren't equipped to deal with that. I think we'd be best off to remove a layer of caching in this code. If this was originally done for performance reasons, then that really seems like a premature optimization. This patch does that -- it removes the rbtrees and the locks that protect them and simply has the code do a request_key call on each call into sid_to_id and id_to_sid. This greatly simplifies this code and should roughly halve the memory utilization from using the idmapping code. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 535 +++++++++------------------------------------------- fs/cifs/cifsacl.h | 28 +-- fs/cifs/cifsfs.c | 1 - fs/cifs/cifsproto.h | 1 - 4 files changed, 99 insertions(+), 466 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 42b3fe981a0a..f4508ee4e80d 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -44,128 +44,6 @@ static const struct cifs_sid sid_user = {1, 2 , {0, 0, 0, 0, 0, 5}, {} }; static const struct cred *root_cred; -static void -shrink_idmap_tree(struct rb_root *root, int nr_to_scan, int *nr_rem, - int *nr_del) -{ - struct rb_node *node; - struct rb_node *tmp; - struct cifs_sid_id *psidid; - - node = rb_first(root); - while (node) { - tmp = node; - node = rb_next(tmp); - psidid = rb_entry(tmp, struct cifs_sid_id, rbnode); - if (nr_to_scan == 0 || *nr_del == nr_to_scan) - ++(*nr_rem); - else { - if (time_after(jiffies, psidid->time + SID_MAP_EXPIRE) - && psidid->refcount == 0) { - rb_erase(tmp, root); - ++(*nr_del); - } else - ++(*nr_rem); - } - } -} - -/* - * Run idmap cache shrinker. - */ -static int -cifs_idmap_shrinker(struct shrinker *shrink, struct shrink_control *sc) -{ - int nr_to_scan = sc->nr_to_scan; - int nr_del = 0; - int nr_rem = 0; - struct rb_root *root; - - root = &uidtree; - spin_lock(&siduidlock); - shrink_idmap_tree(root, nr_to_scan, &nr_rem, &nr_del); - spin_unlock(&siduidlock); - - root = &gidtree; - spin_lock(&sidgidlock); - shrink_idmap_tree(root, nr_to_scan, &nr_rem, &nr_del); - spin_unlock(&sidgidlock); - - root = &siduidtree; - spin_lock(&uidsidlock); - shrink_idmap_tree(root, nr_to_scan, &nr_rem, &nr_del); - spin_unlock(&uidsidlock); - - root = &sidgidtree; - spin_lock(&gidsidlock); - shrink_idmap_tree(root, nr_to_scan, &nr_rem, &nr_del); - spin_unlock(&gidsidlock); - - return nr_rem; -} - -static void -sid_rb_insert(struct rb_root *root, unsigned long cid, - struct cifs_sid_id **psidid, char *typestr) -{ - char *strptr; - struct rb_node *node = root->rb_node; - struct rb_node *parent = NULL; - struct rb_node **linkto = &(root->rb_node); - struct cifs_sid_id *lsidid; - - while (node) { - lsidid = rb_entry(node, struct cifs_sid_id, rbnode); - parent = node; - if (cid > lsidid->id) { - linkto = &(node->rb_left); - node = node->rb_left; - } - if (cid < lsidid->id) { - linkto = &(node->rb_right); - node = node->rb_right; - } - } - - (*psidid)->id = cid; - (*psidid)->time = jiffies - (SID_MAP_RETRY + 1); - (*psidid)->refcount = 0; - - sprintf((*psidid)->sidstr, "%s", typestr); - strptr = (*psidid)->sidstr + strlen((*psidid)->sidstr); - sprintf(strptr, "%ld", cid); - - clear_bit(SID_ID_PENDING, &(*psidid)->state); - clear_bit(SID_ID_MAPPED, &(*psidid)->state); - - rb_link_node(&(*psidid)->rbnode, parent, linkto); - rb_insert_color(&(*psidid)->rbnode, root); -} - -static struct cifs_sid_id * -sid_rb_search(struct rb_root *root, unsigned long cid) -{ - struct rb_node *node = root->rb_node; - struct cifs_sid_id *lsidid; - - while (node) { - lsidid = rb_entry(node, struct cifs_sid_id, rbnode); - if (cid > lsidid->id) - node = node->rb_left; - else if (cid < lsidid->id) - node = node->rb_right; - else /* node found */ - return lsidid; - } - - return NULL; -} - -static struct shrinker cifs_shrinker = { - .shrink = cifs_idmap_shrinker, - .seeks = DEFAULT_SEEKS, -}; - static int cifs_idmap_key_instantiate(struct key *key, struct key_preparsed_payload *prep) { @@ -195,30 +73,39 @@ static struct key_type cifs_idmap_key_type = { .match = user_match, }; -static void -sid_to_str(struct cifs_sid *sidptr, char *sidstr) +static char * +sid_to_key_str(struct cifs_sid *sidptr, unsigned int type) { - int i; + int i, len; unsigned int saval; - char *strptr; + char *sidstr, *strptr; - strptr = sidstr; + /* 3 bytes for prefix */ + sidstr = kmalloc(3 + SID_STRING_BASE_SIZE + + (SID_STRING_SUBAUTH_SIZE * sidptr->num_subauth), + GFP_KERNEL); + if (!sidstr) + return sidstr; - sprintf(strptr, "S-%hhu", sidptr->revision); - strptr = sidstr + strlen(sidstr); + strptr = sidstr; + len = sprintf(strptr, "%cs:S-%hhu", type == SIDOWNER ? 'o' : 'g', + sidptr->revision); + strptr += len; for (i = 0; i < NUM_AUTHS; ++i) { if (sidptr->authority[i]) { - sprintf(strptr, "-%hhu", sidptr->authority[i]); - strptr = sidstr + strlen(sidstr); + len = sprintf(strptr, "-%hhu", sidptr->authority[i]); + strptr += len; } } for (i = 0; i < sidptr->num_subauth; ++i) { saval = le32_to_cpu(sidptr->sub_auth[i]); - sprintf(strptr, "-%u", saval); - strptr = sidstr + strlen(sidstr); + len = sprintf(strptr, "-%u", saval); + strptr += len; } + + return sidstr; } /* @@ -284,184 +171,38 @@ cifs_copy_sid(struct cifs_sid *dst, const struct cifs_sid *src) dst->sub_auth[i] = src->sub_auth[i]; } -static void -id_rb_insert(struct rb_root *root, struct cifs_sid *sidptr, - struct cifs_sid_id **psidid, char *typestr) -{ - int rc; - char *strptr; - struct rb_node *node = root->rb_node; - struct rb_node *parent = NULL; - struct rb_node **linkto = &(root->rb_node); - struct cifs_sid_id *lsidid; - - while (node) { - lsidid = rb_entry(node, struct cifs_sid_id, rbnode); - parent = node; - rc = compare_sids(sidptr, &((lsidid)->sid)); - if (rc > 0) { - linkto = &(node->rb_left); - node = node->rb_left; - } else if (rc < 0) { - linkto = &(node->rb_right); - node = node->rb_right; - } - } - - cifs_copy_sid(&(*psidid)->sid, sidptr); - (*psidid)->time = jiffies - (SID_MAP_RETRY + 1); - (*psidid)->refcount = 0; - - sprintf((*psidid)->sidstr, "%s", typestr); - strptr = (*psidid)->sidstr + strlen((*psidid)->sidstr); - sid_to_str(&(*psidid)->sid, strptr); - - clear_bit(SID_ID_PENDING, &(*psidid)->state); - clear_bit(SID_ID_MAPPED, &(*psidid)->state); - - rb_link_node(&(*psidid)->rbnode, parent, linkto); - rb_insert_color(&(*psidid)->rbnode, root); -} - -static struct cifs_sid_id * -id_rb_search(struct rb_root *root, struct cifs_sid *sidptr) -{ - int rc; - struct rb_node *node = root->rb_node; - struct cifs_sid_id *lsidid; - - while (node) { - lsidid = rb_entry(node, struct cifs_sid_id, rbnode); - rc = compare_sids(sidptr, &((lsidid)->sid)); - if (rc > 0) { - node = node->rb_left; - } else if (rc < 0) { - node = node->rb_right; - } else /* node found */ - return lsidid; - } - - return NULL; -} - -static int -sidid_pending_wait(void *unused) -{ - schedule(); - return signal_pending(current) ? -ERESTARTSYS : 0; -} - static int -id_to_sid(unsigned long cid, uint sidtype, struct cifs_sid *ssid) +id_to_sid(unsigned int cid, uint sidtype, struct cifs_sid *ssid) { - int rc = 0; + int rc; struct key *sidkey; + char desc[3 + 10 + 1]; /* 3 byte prefix + 10 bytes for value + NULL */ const struct cred *saved_cred; - struct cifs_sid *lsid; - struct cifs_sid_id *psidid, *npsidid; - struct rb_root *cidtree; - spinlock_t *cidlock; - - if (sidtype == SIDOWNER) { - cidlock = &siduidlock; - cidtree = &uidtree; - } else if (sidtype == SIDGROUP) { - cidlock = &sidgidlock; - cidtree = &gidtree; - } else - return -EINVAL; - - spin_lock(cidlock); - psidid = sid_rb_search(cidtree, cid); - - if (!psidid) { /* node does not exist, allocate one & attempt adding */ - spin_unlock(cidlock); - npsidid = kzalloc(sizeof(struct cifs_sid_id), GFP_KERNEL); - if (!npsidid) - return -ENOMEM; - - npsidid->sidstr = kmalloc(SID_STRING_MAX, GFP_KERNEL); - if (!npsidid->sidstr) { - kfree(npsidid); - return -ENOMEM; - } - - spin_lock(cidlock); - psidid = sid_rb_search(cidtree, cid); - if (psidid) { /* node happened to get inserted meanwhile */ - ++psidid->refcount; - spin_unlock(cidlock); - kfree(npsidid->sidstr); - kfree(npsidid); - } else { - psidid = npsidid; - sid_rb_insert(cidtree, cid, &psidid, - sidtype == SIDOWNER ? "oi:" : "gi:"); - ++psidid->refcount; - spin_unlock(cidlock); - } - } else { - ++psidid->refcount; - spin_unlock(cidlock); - } - /* - * If we are here, it is safe to access psidid and its fields - * since a reference was taken earlier while holding the spinlock. - * A reference on the node is put without holding the spinlock - * and it is OK to do so in this case, shrinker will not erase - * this node until all references are put and we do not access - * any fields of the node after a reference is put . - */ - if (test_bit(SID_ID_MAPPED, &psidid->state)) { - cifs_copy_sid(ssid, &psidid->sid); - psidid->time = jiffies; /* update ts for accessing */ - goto id_sid_out; - } + rc = snprintf(desc, sizeof(desc), "%ci:%u", + sidtype == SIDOWNER ? 'o' : 'g', cid); + if (rc >= sizeof(desc)) + return -EINVAL; - if (time_after(psidid->time + SID_MAP_RETRY, jiffies)) { + rc = 0; + saved_cred = override_creds(root_cred); + sidkey = request_key(&cifs_idmap_key_type, desc, ""); + if (IS_ERR(sidkey)) { rc = -EINVAL; - goto id_sid_out; - } - - if (!test_and_set_bit(SID_ID_PENDING, &psidid->state)) { - saved_cred = override_creds(root_cred); - sidkey = request_key(&cifs_idmap_key_type, psidid->sidstr, ""); - if (IS_ERR(sidkey)) { - rc = -EINVAL; - cFYI(1, "%s: Can't map and id to a SID", __func__); - } else if (sidkey->datalen < CIFS_SID_BASE_SIZE) { - rc = -EIO; - cFYI(1, "%s: Downcall contained malformed key " - "(datalen=%hu)", __func__, sidkey->datalen); - } else { - lsid = (struct cifs_sid *)sidkey->payload.data; - cifs_copy_sid(&psidid->sid, lsid); - cifs_copy_sid(ssid, &psidid->sid); - set_bit(SID_ID_MAPPED, &psidid->state); - key_put(sidkey); - kfree(psidid->sidstr); - } - psidid->time = jiffies; /* update ts for accessing */ - revert_creds(saved_cred); - clear_bit(SID_ID_PENDING, &psidid->state); - wake_up_bit(&psidid->state, SID_ID_PENDING); - } else { - rc = wait_on_bit(&psidid->state, SID_ID_PENDING, - sidid_pending_wait, TASK_INTERRUPTIBLE); - if (rc) { - cFYI(1, "%s: sidid_pending_wait interrupted %d", - __func__, rc); - --psidid->refcount; - return rc; - } - if (test_bit(SID_ID_MAPPED, &psidid->state)) - cifs_copy_sid(ssid, &psidid->sid); - else - rc = -EINVAL; - } -id_sid_out: - --psidid->refcount; + cFYI(1, "%s: Can't map %cid %u to a SID", __func__, + sidtype == SIDOWNER ? 'u' : 'g', cid); + goto out_revert_creds; + } else if (sidkey->datalen < CIFS_SID_BASE_SIZE) { + rc = -EIO; + cFYI(1, "%s: Downcall contained malformed key " + "(datalen=%hu)", __func__, sidkey->datalen); + goto out_key_put; + } + cifs_copy_sid(ssid, (struct cifs_sid *)sidkey->payload.data); +out_key_put: + key_put(sidkey); +out_revert_creds: + revert_creds(saved_cred); return rc; } @@ -470,111 +211,66 @@ sid_to_id(struct cifs_sb_info *cifs_sb, struct cifs_sid *psid, struct cifs_fattr *fattr, uint sidtype) { int rc; - unsigned long cid; - struct key *idkey; + struct key *sidkey; + char *sidstr; const struct cred *saved_cred; - struct cifs_sid_id *psidid, *npsidid; - struct rb_root *cidtree; - spinlock_t *cidlock; - - if (sidtype == SIDOWNER) { - cid = cifs_sb->mnt_uid; /* default uid, in case upcall fails */ - cidlock = &siduidlock; - cidtree = &uidtree; - } else if (sidtype == SIDGROUP) { - cid = cifs_sb->mnt_gid; /* default gid, in case upcall fails */ - cidlock = &sidgidlock; - cidtree = &gidtree; - } else - return -ENOENT; - - spin_lock(cidlock); - psidid = id_rb_search(cidtree, psid); - - if (!psidid) { /* node does not exist, allocate one & attempt adding */ - spin_unlock(cidlock); - npsidid = kzalloc(sizeof(struct cifs_sid_id), GFP_KERNEL); - if (!npsidid) - return -ENOMEM; - - npsidid->sidstr = kmalloc(SID_STRING_MAX, GFP_KERNEL); - if (!npsidid->sidstr) { - kfree(npsidid); - return -ENOMEM; - } - - spin_lock(cidlock); - psidid = id_rb_search(cidtree, psid); - if (psidid) { /* node happened to get inserted meanwhile */ - ++psidid->refcount; - spin_unlock(cidlock); - kfree(npsidid->sidstr); - kfree(npsidid); - } else { - psidid = npsidid; - id_rb_insert(cidtree, psid, &psidid, - sidtype == SIDOWNER ? "os:" : "gs:"); - ++psidid->refcount; - spin_unlock(cidlock); - } - } else { - ++psidid->refcount; - spin_unlock(cidlock); - } + uid_t fuid = cifs_sb->mnt_uid; + gid_t fgid = cifs_sb->mnt_gid; /* - * If we are here, it is safe to access psidid and its fields - * since a reference was taken earlier while holding the spinlock. - * A reference on the node is put without holding the spinlock - * and it is OK to do so in this case, shrinker will not erase - * this node until all references are put and we do not access - * any fields of the node after a reference is put . + * If we have too many subauthorities, then something is really wrong. + * Just return an error. */ - if (test_bit(SID_ID_MAPPED, &psidid->state)) { - cid = psidid->id; - psidid->time = jiffies; /* update ts for accessing */ - goto sid_to_id_out; + if (unlikely(psid->num_subauth > SID_MAX_SUB_AUTHORITIES)) { + cFYI(1, "%s: %u subauthorities is too many!", __func__, + psid->num_subauth); + return -EIO; } - if (time_after(psidid->time + SID_MAP_RETRY, jiffies)) - goto sid_to_id_out; - - if (!test_and_set_bit(SID_ID_PENDING, &psidid->state)) { - saved_cred = override_creds(root_cred); - idkey = request_key(&cifs_idmap_key_type, psidid->sidstr, ""); - if (IS_ERR(idkey)) - cFYI(1, "%s: Can't map SID to an id", __func__); - else { - cid = *(unsigned long *)idkey->payload.value; - psidid->id = cid; - set_bit(SID_ID_MAPPED, &psidid->state); - key_put(idkey); - kfree(psidid->sidstr); - } - revert_creds(saved_cred); - psidid->time = jiffies; /* update ts for accessing */ - clear_bit(SID_ID_PENDING, &psidid->state); - wake_up_bit(&psidid->state, SID_ID_PENDING); - } else { - rc = wait_on_bit(&psidid->state, SID_ID_PENDING, - sidid_pending_wait, TASK_INTERRUPTIBLE); - if (rc) { - cFYI(1, "%s: sidid_pending_wait interrupted %d", - __func__, rc); - --psidid->refcount; /* decremented without spinlock */ - return rc; - } - if (test_bit(SID_ID_MAPPED, &psidid->state)) - cid = psidid->id; + sidstr = sid_to_key_str(psid, sidtype); + if (!sidstr) + return -ENOMEM; + + saved_cred = override_creds(root_cred); + sidkey = request_key(&cifs_idmap_key_type, sidstr, ""); + if (IS_ERR(sidkey)) { + rc = -EINVAL; + cFYI(1, "%s: Can't map SID %s to a %cid", __func__, sidstr, + sidtype == SIDOWNER ? 'u' : 'g'); + goto out_revert_creds; + } + + /* + * FIXME: Here we assume that uid_t and gid_t are same size. It's + * probably a safe assumption but might be better to check based on + * sidtype. + */ + if (sidkey->datalen < sizeof(uid_t)) { + rc = -EIO; + cFYI(1, "%s: Downcall contained malformed key " + "(datalen=%hu)", __func__, sidkey->datalen); + goto out_key_put; } -sid_to_id_out: - --psidid->refcount; /* decremented without spinlock */ if (sidtype == SIDOWNER) - fattr->cf_uid = cid; + fuid = *(uid_t *)sidkey->payload.value; else - fattr->cf_gid = cid; + fgid = *(gid_t *)sidkey->payload.value; +out_key_put: + key_put(sidkey); +out_revert_creds: + revert_creds(saved_cred); + kfree(sidstr); + + /* + * Note that we return 0 here unconditionally. If the mapping + * fails then we just fall back to using the mnt_uid/mnt_gid. + */ + if (sidtype == SIDOWNER) + fattr->cf_uid = fuid; + else + fattr->cf_gid = fgid; return 0; } @@ -621,17 +317,6 @@ init_cifs_idmap(void) cred->jit_keyring = KEY_REQKEY_DEFL_THREAD_KEYRING; root_cred = cred; - spin_lock_init(&siduidlock); - uidtree = RB_ROOT; - spin_lock_init(&sidgidlock); - gidtree = RB_ROOT; - - spin_lock_init(&uidsidlock); - siduidtree = RB_ROOT; - spin_lock_init(&gidsidlock); - sidgidtree = RB_ROOT; - register_shrinker(&cifs_shrinker); - cFYI(1, "cifs idmap keyring: %d", key_serial(keyring)); return 0; @@ -648,41 +333,9 @@ exit_cifs_idmap(void) key_revoke(root_cred->thread_keyring); unregister_key_type(&cifs_idmap_key_type); put_cred(root_cred); - unregister_shrinker(&cifs_shrinker); cFYI(1, "Unregistered %s key type", cifs_idmap_key_type.name); } -void -cifs_destroy_idmaptrees(void) -{ - struct rb_root *root; - struct rb_node *node; - - root = &uidtree; - spin_lock(&siduidlock); - while ((node = rb_first(root))) - rb_erase(node, root); - spin_unlock(&siduidlock); - - root = &gidtree; - spin_lock(&sidgidlock); - while ((node = rb_first(root))) - rb_erase(node, root); - spin_unlock(&sidgidlock); - - root = &siduidtree; - spin_lock(&uidsidlock); - while ((node = rb_first(root))) - rb_erase(node, root); - spin_unlock(&uidsidlock); - - root = &sidgidtree; - spin_lock(&gidsidlock); - while ((node = rb_first(root))) - rb_erase(node, root); - spin_unlock(&gidsidlock); -} - /* copy ntsd, owner sid, and group sid from a security descriptor to another */ static void copy_sec_desc(const struct cifs_ntsd *pntsd, struct cifs_ntsd *pnntsd, __u32 sidsoffset) diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 249c94f39635..46cd444ea2f2 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -23,7 +23,7 @@ #define _CIFSACL_H -#define NUM_AUTHS 6 /* number of authority fields */ +#define NUM_AUTHS (6) /* number of authority fields */ #define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */ #define NUM_WK_SIDS 7 /* number of well known sids */ #define SIDNAMELENGTH 20 /* long enough for the ones we care about */ @@ -51,15 +51,12 @@ * u32: max 10 bytes in decimal * * "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes - * per number + 1 for '-') + 11 bytes for each subauthority field (10 bytes * per number + 1 for '-') + NULL terminator. + * + * Add 11 bytes for each subauthority field (10 bytes each + 1 for '-') */ -#define SID_STRING_MAX (195) - -#define SID_ID_MAPPED 0 -#define SID_ID_PENDING 1 -#define SID_MAP_EXPIRE (3600 * HZ) /* map entry expires after one hour */ -#define SID_MAP_RETRY (300 * HZ) /* wait 5 minutes for next attempt to map */ +#define SID_STRING_BASE_SIZE (2 + 3 + (4 * NUM_AUTHS) + 1) +#define SID_STRING_SUBAUTH_SIZE (11) /* size of a single subauth string */ struct cifs_ntsd { __le16 revision; /* revision level */ @@ -94,19 +91,4 @@ struct cifs_ace { struct cifs_sid sid; /* ie UUID of user or group who gets these perms */ } __attribute__((packed)); -struct cifs_wksid { - struct cifs_sid cifssid; - char sidname[SIDNAMELENGTH]; -} __attribute__((packed)); - -struct cifs_sid_id { - unsigned int refcount; /* increment with spinlock, decrement without */ - unsigned long id; - unsigned long time; - unsigned long state; - char *sidstr; - struct rb_node rbnode; - struct cifs_sid sid; -}; - #endif /* _CIFSACL_H */ diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 273b34904d5b..c6e32f22fbd3 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -1204,7 +1204,6 @@ exit_cifs(void) unregister_filesystem(&cifs_fs_type); cifs_dfs_release_automount_timer(); #ifdef CONFIG_CIFS_ACL - cifs_destroy_idmaptrees(); exit_cifs_idmap(); #endif #ifdef CONFIG_CIFS_UPCALL diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index a152f3645b09..1988c1baa224 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -58,7 +58,6 @@ do { \ } while (0) extern int init_cifs_idmap(void); extern void exit_cifs_idmap(void); -extern void cifs_destroy_idmaptrees(void); extern char *build_path_from_dentry(struct dentry *); extern char *cifs_build_path_to_root(struct smb_vol *vol, struct cifs_sb_info *cifs_sb, -- cgit v1.2.3 From 7ee0b4c635c091eb3c805977ba886bae2fd33f0c Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 3 Dec 2012 06:05:31 -0500 Subject: cifs: fix hardcoded default security descriptor length It was hardcoded to 192 bytes, which was not enough when the max number of subauthorities went to 15. Redefine this constant in terms of sizeof the structs involved, and rename it for better clarity. While we're at it, remove a couple more unused constants from cifsacl.h. Reviewed-by: Shirish Pargaonkar Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 2 +- fs/cifs/cifsacl.h | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index b0b114acdece..08b4d5022686 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -1008,7 +1008,7 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 nmode, * memory for the smb header, set security descriptor request security * descriptor parameters, and secuirty descriptor itself */ - secdesclen = max_t(u32, secdesclen, DEFSECDESCLEN); + secdesclen = max_t(u32, secdesclen, DEFAULT_SEC_DESC_LEN); pnntsd = kmalloc(secdesclen, GFP_KERNEL); if (!pnntsd) { cERROR(1, "Unable to allocate security descriptor"); diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index 46cd444ea2f2..a445405f80d0 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -25,9 +25,6 @@ #define NUM_AUTHS (6) /* number of authority fields */ #define SID_MAX_SUB_AUTHORITIES (15) /* max number of sub authority fields */ -#define NUM_WK_SIDS 7 /* number of well known sids */ -#define SIDNAMELENGTH 20 /* long enough for the ones we care about */ -#define DEFSECDESCLEN 192 /* sec desc len contaiting a dacl with three aces */ #define READ_BIT 0x4 #define WRITE_BIT 0x2 @@ -42,6 +39,14 @@ #define SIDOWNER 1 #define SIDGROUP 2 +/* + * Security Descriptor length containing DACL with 3 ACEs (one each for + * owner, group and world). + */ +#define DEFAULT_SEC_DESC_LEN (sizeof(struct cifs_ntsd) + \ + sizeof(struct cifs_acl) + \ + (sizeof(struct cifs_ace) * 3)) + /* * Maximum size of a string representation of a SID: * -- cgit v1.2.3 From 193cdd8a293007d1a1ad252cf66b2dc5b793d2d0 Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Mon, 10 Dec 2012 06:10:44 -0500 Subject: cifs: fix SID binary to string conversion The authority fields are supposed to be represented by a single 48-bit value. It's also supposed to represent the value as hex if it's equal to or greater than 2^32. This is documented in MS-DTYP, section 2.4.2.1. Also, fix up the max string length to account for this fix. Acked-by: Pavel Shilovsky Signed-off-by: Jeff Layton Signed-off-by: Steve French --- fs/cifs/cifsacl.c | 25 +++++++++++++++++++------ fs/cifs/cifsacl.h | 8 +++++--- 2 files changed, 24 insertions(+), 9 deletions(-) (limited to 'fs/cifs/cifsacl.h') diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c index 8dd9212ffef5..75c1ee699143 100644 --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -94,6 +94,7 @@ sid_to_key_str(struct cifs_sid *sidptr, unsigned int type) int i, len; unsigned int saval; char *sidstr, *strptr; + unsigned long long id_auth_val; /* 3 bytes for prefix */ sidstr = kmalloc(3 + SID_STRING_BASE_SIZE + @@ -107,12 +108,24 @@ sid_to_key_str(struct cifs_sid *sidptr, unsigned int type) sidptr->revision); strptr += len; - for (i = 0; i < NUM_AUTHS; ++i) { - if (sidptr->authority[i]) { - len = sprintf(strptr, "-%hhu", sidptr->authority[i]); - strptr += len; - } - } + /* The authority field is a single 48-bit number */ + id_auth_val = (unsigned long long)sidptr->authority[5]; + id_auth_val |= (unsigned long long)sidptr->authority[4] << 8; + id_auth_val |= (unsigned long long)sidptr->authority[3] << 16; + id_auth_val |= (unsigned long long)sidptr->authority[2] << 24; + id_auth_val |= (unsigned long long)sidptr->authority[1] << 32; + id_auth_val |= (unsigned long long)sidptr->authority[0] << 48; + + /* + * MS-DTYP states that if the authority is >= 2^32, then it should be + * expressed as a hex value. + */ + if (id_auth_val <= UINT_MAX) + len = sprintf(strptr, "-%llu", id_auth_val); + else + len = sprintf(strptr, "-0x%llx", id_auth_val); + + strptr += len; for (i = 0; i < sidptr->num_subauth; ++i) { saval = le32_to_cpu(sidptr->sub_auth[i]); diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h index a445405f80d0..4f3884835267 100644 --- a/fs/cifs/cifsacl.h +++ b/fs/cifs/cifsacl.h @@ -55,12 +55,14 @@ * u8: max 3 bytes in decimal * u32: max 10 bytes in decimal * - * "S-" + 3 bytes for version field + 4 bytes for each authority field (3 bytes - * per number + 1 for '-') + NULL terminator. + * "S-" + 3 bytes for version field + 15 for authority field + NULL terminator + * + * For authority field, max is when all 6 values are non-zero and it must be + * represented in hex. So "-0x" + 12 hex digits. * * Add 11 bytes for each subauthority field (10 bytes each + 1 for '-') */ -#define SID_STRING_BASE_SIZE (2 + 3 + (4 * NUM_AUTHS) + 1) +#define SID_STRING_BASE_SIZE (2 + 3 + 15 + 1) #define SID_STRING_SUBAUTH_SIZE (11) /* size of a single subauth string */ struct cifs_ntsd { -- cgit v1.2.3