[md PATCH 0/4] Minor 'cred' improvements prepare for NFS conversion

[md PATCH 0/4] Minor 'cred' improvements prepare for NFS conversion

[NeilBrown]

Hi,
 NFS and SUNRPC have an internal "rpc_cred" which plays two distinct
 roles, one of which is much the same as 'struct cred' (which didn't
 exist when rpc_cred was created).
 I want to replace that usage of rpc_cred with 'struct cred'.
 Doing so requires some minor improvements to cred.c and cred.h as
 follows.

 It isn't clear to me who "maintains" cred.c and cred.h, so I'm hoping
 that Andrew Morton will take these (if no-one complains).
 Alternately if I got one or two credible "Acked-by"s, they could go
 upstream through the NFS tree when the rest of the patches are ready.

Thanks,
NeilBrown


---

NeilBrown (4):
      cred: add cred_fscmp() for comparing creds.
      cred: add get_cred_rcu()
      cred: export get_task_cred().
      cred: allow get_cred() and put_cred() to be given NULL.


 include/linux/cred.h |   26 ++++++++++++++++++----
 kernel/cred.c        |   58 +++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 6 deletions(-)

--
Signature

[PATCH 4/4] cred: allow get_cred() and put_cred() to be given NULL.

[NeilBrown]

It is common practice of helps like this so silently,
accept a NULL pointer.
get_rpccred() and put_rpccred() used by NFS act this way
and using the same interface will ease the conversion
for NFS, and simplify the resulting code.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 include/linux/cred.h |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index 69ed76f7d49f..c9ba3c8747e9 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -232,7 +232,7 @@ static inline struct cred *get_new_cred(struct cred *cred)
  * @cred: The credentials to reference
  *
  * Get a reference on the specified set of credentials.  The caller must
- * release the reference.
+ * release the reference.  If %NULL is passed, it is returned with no action.
  *
  * This is used to deal with a committed set of credentials.  Although the
  * pointer is const, this will temporarily discard the const and increment the
@@ -243,6 +243,8 @@ static inline struct cred *get_new_cred(struct cred *cred)
 static inline const struct cred *get_cred(const struct cred *cred)
 {
 	struct cred *nonconst_cred = (struct cred *) cred;
+	if (!cred)
+		return cred;
 	validate_creds(cred);
 	return get_new_cred(nonconst_cred);
 }
@@ -263,7 +265,7 @@ static inline const struct cred *get_cred_rcu(const struct cred *cred)
  * @cred: The credentials to release
  *
  * Release a reference to a set of credentials, deleting them when the last ref
- * is released.
+ * is released.  If %NULL is passed, nothing is done.
  *
  * This takes a const pointer to a set of credentials because the credentials
  * on task_struct are attached by const pointers to prevent accidental
@@ -273,9 +275,11 @@ static inline void put_cred(const struct cred *_cred)
 {
 	struct cred *cred = (struct cred *) _cred;
 
-	validate_creds(cred);
-	if (atomic_dec_and_test(&(cred)->usage))
-		__put_cred(cred);
+	if (cred) {
+		validate_creds(cred);
+		if (atomic_dec_and_test(&(cred)->usage))
+			__put_cred(cred);
+	}
 }
 
 /**

[PATCH 3/4] cred: export get_task_cred().

[NeilBrown]

There is no reason that modules should not be able
to use this, and NFS will need it when converted to
use 'struct cred'.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 kernel/cred.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/cred.c b/kernel/cred.c
index f11aa4e0d2b9..3e43bde1fd3c 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -200,6 +200,7 @@ const struct cred *get_task_cred(struct task_struct *task)
 	rcu_read_unlock();
 	return cred;
 }
+EXPORT_SYMBOL(get_task_cred);
 
 /*
  * Allocate blank credentials, such that the credentials can be filled in at a

[PATCH 2/4] cred: add get_cred_rcu()

[NeilBrown]

Sometimes we want to opportunistically get a
ref to a cred in an rcu_read_lock protected section.
get_task_cred() does this, and NFS does as similar thing
with its own credential structures.
To prepare for NFS converting to use 'struct cred' more
uniformly, define get_cred_rcu(), and use it in
get_task_cred().

Signed-off-by: NeilBrown <neilb@suse.com>
---
 include/linux/cred.h |   11 +++++++++++
 kernel/cred.c        |    2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index 6dd51e503f23..69ed76f7d49f 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -247,6 +247,17 @@ static inline const struct cred *get_cred(const struct cred *cred)
 	return get_new_cred(nonconst_cred);
 }
 
+static inline const struct cred *get_cred_rcu(const struct cred *cred)
+{
+	struct cred *nonconst_cred = (struct cred *) cred;
+	if (!cred)
+		return NULL;
+	if (!atomic_inc_not_zero(&nonconst_cred->usage))
+		return NULL;
+	validate_creds(cred);
+	return cred;
+}
+
 /**
  * put_cred - Release a reference to a set of credentials
  * @cred: The credentials to release
diff --git a/kernel/cred.c b/kernel/cred.c
index 4ce75c6fb752..f11aa4e0d2b9 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -195,7 +195,7 @@ const struct cred *get_task_cred(struct task_struct *task)
 	do {
 		cred = __task_cred((task));
 		BUG_ON(!cred);
-	} while (!atomic_inc_not_zero(&((struct cred *)cred)->usage));
+	} while (!get_cred_rcu(cred));
 
 	rcu_read_unlock();
 	return cred;

[PATCH 1/4] cred: add cred_fscmp() for comparing creds.

[NeilBrown]

NFS needs to compare to credentials, to see if they can
be treated the same w.r.t. filesystem access.  Sometimes
an ordering is needed when credentials are used as a key
to an rbtree.
NFS current has its own private credential management from
before 'struct cred' existed.  To move it over to more consistent
use of 'struct cred' we need a comparison function.
This patch adds that function.

Signed-off-by: NeilBrown <neilb@suse.com>
---
 include/linux/cred.h |    1 +
 kernel/cred.c        |   55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index 631286535d0f..6dd51e503f23 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -164,6 +164,7 @@ extern int change_create_files_as(struct cred *, struct inode *);
 extern int set_security_override(struct cred *, u32);
 extern int set_security_override_from_ctx(struct cred *, const char *);
 extern int set_create_files_as(struct cred *, struct inode *);
+extern int cred_fscmp(const struct cred *, const struct cred *);
 extern void __init cred_init(void);
 
 /*
diff --git a/kernel/cred.c b/kernel/cred.c
index ecf03657e71c..4ce75c6fb752 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -19,6 +19,7 @@
 #include <linux/security.h>
 #include <linux/binfmts.h>
 #include <linux/cn_proc.h>
+#include <linux/uidgid.h>
 
 #if 0
 #define kdebug(FMT, ...)						\
@@ -564,6 +565,60 @@ void revert_creds(const struct cred *old)
 }
 EXPORT_SYMBOL(revert_creds);
 
+/**
+ * cred_fscmp - Compare to credentials with respect to filesystem access.
+ * @a: The first credential
+ * @b: The second credential
+ *
+ * cred_cmp() will return zero if both credentials have the same
+ * fsuid, fsgid, and supplementary groups.  That is, if they will both
+ * provide the same access to files based on mode/uid/gid.
+ * If the credentials are different, then either -1 or 1 will
+ * be returned depending on whether @a comes before or after @b
+ * respectively in an arbitrary, but stable, ordering of credentials.
+ *
+ * Return: -1, 0, or 1 depending on comparison
+ */
+int cred_fscmp(const struct cred *a, const struct cred *b)
+{
+	struct group_info *ga, *gb;
+	int g;
+
+	if (a == b)
+		return 0;
+	if (uid_lt(a->fsuid, b->fsuid))
+		return -1;
+	if (uid_gt(a->fsuid, b->fsuid))
+		return 1;
+
+	if (gid_lt(a->fsgid, b->fsgid))
+		return -1;
+	if (gid_gt(a->fsgid, b->fsgid))
+		return 1;
+
+	ga = a->group_info;
+	gb = b->group_info;
+	if (ga == gb)
+		return 0;
+	if (ga == NULL)
+		return -1;
+	if (gb == NULL)
+		return 1;
+	if (ga->ngroups < gb->ngroups)
+		return -1;
+	if (ga->ngroups > gb->ngroups)
+		return 1;
+
+	for (g = 0; g < ga->ngroups; g++) {
+		if (gid_lt(ga->gid[g], gb->gid[g]))
+			return -1;
+		if (gid_gt(ga->gid[g], gb->gid[g]))
+			return 1;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(cred_fscmp);
+
 /*
  * initialise the credentials stuff
  */

Re: [md PATCH 0/4] Minor 'cred' improvements prepare for NFS conversion

[Anna Schumaker]

On 01/31/2018 12:15 AM, NeilBrown wrote:
> Hi,
>  NFS and SUNRPC have an internal "rpc_cred" which plays two distinct
>  roles, one of which is much the same as 'struct cred' (which didn't
>  exist when rpc_cred was created).
>  I want to replace that usage of rpc_cred with 'struct cred'.
>  Doing so requires some minor improvements to cred.c and cred.h as
>  follows.
> 
>  It isn't clear to me who "maintains" cred.c and cred.h, so I'm hoping
>  that Andrew Morton will take these (if no-one complains).
>  Alternately if I got one or two credible "Acked-by"s, they could go
>  upstream through the NFS tree when the rest of the patches are ready.

Neil sent these patches out a while ago, and I haven't heard any acks for them yet.  Does anybody have a problem with me taking them through the NFS tree for 4.17?

Anna

> 
> Thanks,
> NeilBrown
> 
> 
> ---
> 
> NeilBrown (4):
>       cred: add cred_fscmp() for comparing creds.
>       cred: add get_cred_rcu()
>       cred: export get_task_cred().
>       cred: allow get_cred() and put_cred() to be given NULL.
> 
> 
>  include/linux/cred.h |   26 ++++++++++++++++++----
>  kernel/cred.c        |   58 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 78 insertions(+), 6 deletions(-)
> 
> --
> Signature
>