Re: [PATCH 2/2] cifs: Invoke id mapping functions (try #8)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon,  7 Mar 2011 22:47:07 -0600
shirishpargaonkar@xxxxxxxxx wrote:

> From: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> 
> rb tree search and insertion routines.
> 
> A SID which needs to be mapped, is looked up in one of the rb trees
> depending whether SID is either owner or group SID.
> If found in the tree, a (mapped) id from that node is assigned to
> uid or gid as appropriate.  If upcall returns an error, mapping
> is retried after a timeout period.
> 
> Otherwise, an node is inserted in the tree and a mapping id for
> that SID is looked-up with the help of Winbind via upcall mechanism.
> If, for whatever reasons, winbind calls return error, SID for an
> owner is mapped to 'nobody' and SID for a group is mapped to 'nogroup'.
> 
> To map a SID, which can be either a Owner SID or a Group SID, key
> description starts with the string "os" or "gs" followed by SID converted
> to a string. Without "os" or "gs", cifs.upcall does not know whether
> SID needs to be mapped to either an uid or a gid.
> 
> Nodes in rb tree as fields to prevent multiple upcalls for
> a SID.  Adding and removing nodes is done within global locks.
> shrinker routine does not prune a node if mapping for that node
> is either pending or or node was recently created (before timeout
> period expires and node could be purged). The timeout period should
> be long enough at least for an upcall to return back to cifs.
> 
> For now, cifs.upcall is only used to map a SID to an id (uid or gid) but
> it would be used to obtain an SID (string) for an id.
> 
> 
> Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@xxxxxxxxx>
> ---
>  fs/cifs/cifsacl.c |  295 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  fs/cifs/cifsacl.h |   23 ++++
>  2 files changed, 294 insertions(+), 24 deletions(-)
> 
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index fc1ec64..e560b04 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -54,7 +54,31 @@ static const struct cifs_sid sid_authusers = {
>  /* group users */
>  static const struct cifs_sid sid_user = {1, 2 , {0, 0, 0, 0, 0, 5}, {} };
>  
> -static const struct cred *root_cred;
> +const struct cred *root_cred;
> +
> +void
> +shrink_idmap(struct rb_root *root, int nr_to_scan, int *nr_rem)
> +{
> +	int nr_del = 0;
> +	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 (time_after(psidid->time + SID_MAP_EXPIRE, jiffies) ||
> +				test_bit(SID_ID_PENDING, &psidid->state) ||
> +				(nr_to_scan == 0) || (nr_del == nr_to_scan))
> +			++(*nr_rem);
> +		else {
> +			rb_erase(tmp, root);
> +			++nr_del;
> +		}
> +	}
> +}
>  
>  /*
>   * Run idmap cache shrinker.
> @@ -62,11 +86,23 @@ static const struct cred *root_cred;
>  static int
>  cifs_idmap_shrinker(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask)
>  {
> -	/* Use a pruning scheme in a subsequent patch instead */
> -	cifs_destroy_idmaptrees();
> -	return 0;
> +	int nr_rem = 0;
> +	struct rb_root *root;
> +
> +	root = &uidtree;
> +	spin_lock(&siduidlock);
> +	shrink_idmap(root, nr_to_scan, &nr_rem);
> +	spin_unlock(&siduidlock);
> +
> +	root = &gidtree;
> +	spin_lock(&sidgidlock);
> +	shrink_idmap(root, nr_to_scan, &nr_rem);
> +	spin_unlock(&sidgidlock);
> +
> +	return nr_rem;
>  }
>  
> +
>  static struct shrinker cifs_shrinker = {
>  	.shrink = cifs_idmap_shrinker,
>  	.seeks = DEFAULT_SEEKS,
> @@ -92,7 +128,6 @@ cifs_idmap_key_destroy(struct key *key)
>  	kfree(key->payload.data);
>  }
>  
> -static
>  struct key_type cifs_idmap_key_type = {
>  	.name        = "cifs.cifs_idmap",
>  	.instantiate = cifs_idmap_key_instantiate,
> @@ -101,6 +136,192 @@ struct key_type cifs_idmap_key_type = {
>  	.match       = user_match,
>  };
>  
> +static int
> +id_rb_search_insert(struct rb_root *root, struct cifs_sid *sidptr,
> +		struct cifs_sid_id **psidid, struct cifs_sid_id **epsidid)
> +{
> +	int rc = 1;
> +	struct rb_node *node = root->rb_node;
> +	struct rb_node *parent = NULL;
> +	struct rb_node **linkto = &(root->rb_node);
> +
> +	while (node) {
> +		*epsidid = rb_entry(node, struct cifs_sid_id, rbnode);
> +		parent = node;
> +		rc = compare_sids(sidptr, &((*epsidid)->sid));
> +		if (rc > 0) {
> +			linkto = &(node->rb_left);
> +			node = node->rb_left;
> +		} else if (rc < 0) {
> +			linkto = &(node->rb_right);
> +			node = node->rb_right;
> +		} else { /* node found */
> +			if (test_bit(SID_ID_UNMAPPED, &(*epsidid)->state) &&
> +				!time_after((*epsidid)->retry + SID_MAP_RETRY,
> +						jiffies)) {
> +				/* retry mapping this unmapped SID */
> +				if (!test_and_set_bit(SID_ID_PENDING,
> +						&(*epsidid)->state))
> +					return 1;
> +			}
> +			/*
> +			 * Found a node with either SID mapped to an id
> +			 * or a node with unmapped SID but somebody else
> +			 * get to map it.
> +			 */
> +			return 0;
> +		}
> +	}
> +	*epsidid = NULL;
> +
> +	if (rc) { /* node not found, rc is not 0 */
> +		memcpy(&(*psidid)->sid, sidptr, sizeof(struct cifs_sid));
> +		set_bit(SID_ID_PENDING, &(*psidid)->state);
> +		set_bit(SID_ID_UNMAPPED, &(*psidid)->state);
> +
> +		rb_link_node(&(*psidid)->rbnode, parent, linkto);
> +		rb_insert_color(&(*psidid)->rbnode, root);
> +	}
> +
> +	return rc;
> +}
> +
> +static void
> +sid_to_str(struct cifs_sid *sidptr, char *sidstr)
> +{
> +	int i;
> +	unsigned long saval;
> +	char *strptr;
> +
> +	strptr = sidstr;
> +
> +	sprintf(strptr, "%s", "S");
> +	strptr = sidstr + strlen(sidstr);
> +
> +	sprintf(strptr, "-%d", sidptr->revision);
> +	strptr = sidstr + strlen(sidstr);
> +
> +	for (i = 0; i < 6; ++i) {
> +		if (sidptr->authority[i]) {
> +			sprintf(strptr, "-%d", sidptr->authority[i]);
> +			strptr = sidstr + strlen(sidstr);
> +		}
> +	}
> +
> +	for (i = 0; i < sidptr->num_subauth; ++i) {
> +		saval = le32_to_cpu(sidptr->sub_auth[i]);
> +		sprintf(strptr, "-%ld", saval);
> +		strptr = sidstr + strlen(sidstr);
> +	}
> +}
> +
> +static int
> +cifs_sid_pending_wait(void *unused)
> +{
> +	schedule();
> +	return signal_pending(current) ? -ERESTARTSYS : 0;
> +}
> +
> +static int
> +sid_to_id(struct cifs_sb_info *cifs_sb, struct cifs_sid *psid,
> +		struct cifs_fattr *fattr, uint sidtype)
> +{
> +	int rc;
> +	unsigned long id;
> +	char *strptr;
> +	char *sidstr;
> +	struct key *idkey;
> +	const struct cred *saved_cred;
> +	struct cifs_sid_id *psidid;
> +	struct cifs_sid_id *epsidid = NULL;
> +
> +	if (sidtype != SIDOWNER && sidtype != SIDGROUP)
> +		return -ENOENT;
> +
> +	psidid = kzalloc(sizeof(struct cifs_sid_id), GFP_KERNEL);
> +	if (!psidid)
> +		return -ENOMEM;
> +

So you're going to do a memory allocation every time you go through
this function? That sounds like a bad idea. You really don't want to
penalize the common case where you have an entry already in the tree.

This is what you want to do:

1) look for an entry. If it exists return it

2) if it doesn't exist, do an allocation (outside the spinlock) for an
   entry that can be inserted into the tree.

3) look for the entry again, if it now exists (meaning that you raced
   with another insertion), then simply free the allocation you just
   did.

4) Otherwise, stick the new entry into the tree, mark it pending and
   go off and populate it

...this is almost exactly the same thing that cifs_sb_tlink does.
Perhaps you should base your code on that?

> +	if (sidtype == SIDOWNER) {
> +		id = cifs_sb->mnt_uid;
> +		spin_lock(&siduidlock);
> +		rc = id_rb_search_insert(&uidtree, psid, &psidid, &epsidid);
> +		spin_unlock(&siduidlock);
> +	} else {
> +		id = cifs_sb->mnt_gid;
> +		spin_lock(&sidgidlock);
> +		rc = id_rb_search_insert(&gidtree, psid, &psidid, &epsidid);
> +		spin_unlock(&sidgidlock);
> +	}
> +
> +	if (!epsidid)
> +		epsidid = psidid;
> +	else
> +		kfree(psidid);
> +
> +	if (rc) {
> +		sidstr = kzalloc(SIDLEN, GFP_KERNEL);
> +		if (!sidstr)
> +			rc = -ENOMEM;
> +
> +		/*
> +		 * either a new node was created or an existing node
> +		 * does not have a SID mapped to an id, so make an upcall.
> +		 */
> +		if (sidtype == SIDOWNER)
> +			sprintf(sidstr, "%s", "os:");
> +		else if (sidtype == SIDGROUP)
> +			sprintf(sidstr, "%s", "gs:");
> +
> +		strptr = sidstr + strlen(sidstr);
> +		sid_to_str(psid, strptr);
> +
> +		saved_cred = override_creds(root_cred);
> +		idkey = request_key(&cifs_idmap_key_type, sidstr, "");
> +		if (IS_ERR(idkey))
> +			cFYI(1, "%s: Can't map SID to an id", __func__);
> +		else {
> +			id = *(unsigned long *)idkey->payload.value;
> +			key_put(idkey);
> +			clear_bit(SID_ID_UNMAPPED, &psidid->state);
> +		}
> +		epsidid->time = jiffies;
> +		epsidid->retry = jiffies;
> +		epsidid->id = id;
> +		clear_bit(SID_ID_PENDING, &epsidid->state);
> +		revert_creds(saved_cred);
> +		kfree(sidstr);
> +		rc = 0;
> +	} else {
> +		/*
> +		 * eitehr an existing node with SID mapped or a node for
> +		 * whom its SID is being mapped (in which case, wait).
> +		 */
> +		rc = wait_on_bit(&epsidid->state, SID_ID_PENDING,
> +				cifs_sid_pending_wait, TASK_INTERRUPTIBLE);
> +		if (rc) {
> +			cERROR(1, "%s: cifs_sid_pending_wait interrupted %d",
> +					__func__, rc);
> +			return rc;
> +		}
> +
> +		/*
> +		 * No point in checking whether state has bit SID_ID_UNMAPPED
> +		 * set. If SID_ID_UNMAPPED state bit is not set, upcall must
> +		 * have returned an error.  No point in making an upcall here
> +		 * again right away, it is likely to fail.
> +		 */
> +		id = epsidid->id;
> +	}
> +
> +	if (sidtype == SIDOWNER)
> +		fattr->cf_uid = id;
> +	else
> +		fattr->cf_gid = id;
> +
> +	return 0;
> +}
> +
>  int
>  init_cifs_idmap(void)
>  {
> @@ -242,16 +463,24 @@ int compare_sids(const struct cifs_sid *ctsid, const struct cifs_sid *cwsid)
>  	int num_subauth, num_sat, num_saw;
>  
>  	if ((!ctsid) || (!cwsid))
> -		return 0;
> +		return 1;
>  
>  	/* compare the revision */
> -	if (ctsid->revision != cwsid->revision)
> -		return 0;
> +	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 < 6; ++i) {
> -		if (ctsid->authority[i] != cwsid->authority[i])
> -			return 0;
> +		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 */
> @@ -260,12 +489,16 @@ int compare_sids(const struct cifs_sid *ctsid, const struct cifs_sid *cwsid)
>  	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])
> -				return 0;
> +			if (ctsid->sub_auth[i] != cwsid->sub_auth[i]) {
> +				if (ctsid->sub_auth[i] > cwsid->sub_auth[i])
> +					return 1;
> +				else
> +					return -1;
> +			}
>  		}
>  	}
>  
> -	return 1; /* sids compare/match */
> +	return 0; /* sids compare/match */
>  }
>  
>  
> @@ -520,22 +753,22 @@ static void parse_dacl(struct cifs_acl *pdacl, char *end_of_acl,
>  #ifdef CONFIG_CIFS_DEBUG2
>  			dump_ace(ppace[i], end_of_acl);
>  #endif
> -			if (compare_sids(&(ppace[i]->sid), pownersid))
> +			if (compare_sids(&(ppace[i]->sid), pownersid) == 0)
>  				access_flags_to_mode(ppace[i]->access_req,
>  						     ppace[i]->type,
>  						     &fattr->cf_mode,
>  						     &user_mask);
> -			if (compare_sids(&(ppace[i]->sid), pgrpsid))
> +			if (compare_sids(&(ppace[i]->sid), pgrpsid) == 0)
>  				access_flags_to_mode(ppace[i]->access_req,
>  						     ppace[i]->type,
>  						     &fattr->cf_mode,
>  						     &group_mask);
> -			if (compare_sids(&(ppace[i]->sid), &sid_everyone))
> +			if (compare_sids(&(ppace[i]->sid), &sid_everyone) == 0)
>  				access_flags_to_mode(ppace[i]->access_req,
>  						     ppace[i]->type,
>  						     &fattr->cf_mode,
>  						     &other_mask);
> -			if (compare_sids(&(ppace[i]->sid), &sid_authusers))
> +			if (compare_sids(&(ppace[i]->sid), &sid_authusers) == 0)
>  				access_flags_to_mode(ppace[i]->access_req,
>  						     ppace[i]->type,
>  						     &fattr->cf_mode,
> @@ -613,10 +846,10 @@ static int parse_sid(struct cifs_sid *psid, char *end_of_acl)
>  
>  
>  /* Convert CIFS ACL to POSIX form */
> -static int parse_sec_desc(struct cifs_ntsd *pntsd, int acl_len,
> -			  struct cifs_fattr *fattr)
> +static int parse_sec_desc(struct cifs_sb_info *cifs_sb,
> +		struct cifs_ntsd *pntsd, int acl_len, struct cifs_fattr *fattr)
>  {
> -	int rc;
> +	int rc = 0;
>  	struct cifs_sid *owner_sid_ptr, *group_sid_ptr;
>  	struct cifs_acl *dacl_ptr; /* no need for SACL ptr */
>  	char *end_of_acl = ((char *)pntsd) + acl_len;
> @@ -638,12 +871,26 @@ static int parse_sec_desc(struct cifs_ntsd *pntsd, int acl_len,
>  		 le32_to_cpu(pntsd->sacloffset), dacloffset);
>  /*	cifs_dump_mem("owner_sid: ", owner_sid_ptr, 64); */
>  	rc = parse_sid(owner_sid_ptr, end_of_acl);
> -	if (rc)
> +	if (rc) {
> +		cFYI(1, "%s: Error %d parsing Owner SID", __func__, rc);
> +		return rc;
> +	}
> +	rc = sid_to_id(cifs_sb, owner_sid_ptr, fattr, SIDOWNER);
> +	if (rc) {
> +		cFYI(1, "%s: Error %d mapping Owner SID to uid", __func__, rc);
>  		return rc;
> +	}
>  
>  	rc = parse_sid(group_sid_ptr, end_of_acl);
> -	if (rc)
> +	if (rc) {
> +		cFYI(1, "%s: Error %d mapping Owner SID to gid", __func__, rc);
>  		return rc;
> +	}
> +	rc = sid_to_id(cifs_sb, group_sid_ptr, fattr, SIDGROUP);
> +	if (rc) {
> +		cFYI(1, "%s: Error %d mapping Group SID to gid", __func__, rc);
> +		return rc;
> +	}
>  
>  	if (dacloffset)
>  		parse_dacl(dacl_ptr, end_of_acl, owner_sid_ptr,
> @@ -658,7 +905,7 @@ static int parse_sec_desc(struct cifs_ntsd *pntsd, int acl_len,
>  	memcpy((void *)(&(cifscred->gsid)), (void *)group_sid_ptr,
>  			sizeof(struct cifs_sid)); */
>  
> -	return 0;
> +	return rc;
>  }
>  
>  
> @@ -865,7 +1112,7 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
>  		rc = PTR_ERR(pntsd);
>  		cERROR(1, "%s: error %d getting sec desc", __func__, rc);
>  	} else {
> -		rc = parse_sec_desc(pntsd, acllen, fattr);
> +		rc = parse_sec_desc(cifs_sb, pntsd, acllen, fattr);
>  		kfree(pntsd);
>  		if (rc)
>  			cERROR(1, "parse sec desc failed rc = %d", rc);
> diff --git a/fs/cifs/cifsacl.h b/fs/cifs/cifsacl.h
> index 025e943..5f075c0 100644
> --- a/fs/cifs/cifsacl.h
> +++ b/fs/cifs/cifsacl.h
> @@ -39,6 +39,15 @@
>  #define ACCESS_ALLOWED	0
>  #define ACCESS_DENIED	1
>  
> +#define SIDOWNER 1
> +#define SIDGROUP 2
> +#define SIDLEN 150 /* S- 1 revision- 6 authorities- max 5 sub authorities */
> +
> +#define SID_ID_PENDING 0
> +#define SID_ID_UNMAPPED 1
> +#define SID_MAP_EXPIRE 1000 /* enough for an upcall to return? */
> +#define SID_MAP_RETRY 100 /* retry mapping in case prior attempt failed */
> +
>  struct cifs_ntsd {
>  	__le16 revision; /* revision level */
>  	__le16 type;
> @@ -74,6 +83,20 @@ struct cifs_wksid {
>  	char sidname[SIDNAMELENGTH];
>  } __attribute__((packed));
>  
> +struct cifs_sid_id {
> +	struct rb_node rbnode;
> +	struct cifs_sid sid;
> +	unsigned long state;
> +	unsigned long id;
> +	unsigned long time;
> +	unsigned long retry;
> +};
> +
> +#ifdef __KERNEL__
> +extern struct key_type cifs_idmap_key_type;
> +extern const struct cred *root_cred;
> +#endif /* KERNEL */
> +
>  extern int match_sid(struct cifs_sid *);
>  extern int compare_sids(const struct cifs_sid *, const struct cifs_sid *);
>  


-- 
Jeff Layton <jlayton@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux