fix rcu annotations noise in cred.h

fix rcu annotations noise in cred.h

[Al Viro]

task->cred is declared as __rcu, and access to other tasks' ->cred is,
indeed, protected.  Access to current->cred does not need rcu_dereference()
at all, since only the task itself can change its ->cred.  sparse, of
course, has no way of knowing that...

Add force-cast in current_cred(), make current_fsuid() et.al. use it.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/include/linux/cred.h b/include/linux/cred.h
index 48e82af..98f46ef 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -265,10 +265,11 @@ static inline void put_cred(const struct cred *_cred)
 /**
  * current_cred - Access the current task's subjective credentials
  *
- * Access the subjective credentials of the current task.
+ * Access the subjective credentials of the current task.  RCU-safe,
+ * since nobody else can modify it.
  */
 #define current_cred() \
-	(current->cred)
+	(*(__force struct cred **)&current->cred)
 
 /**
  * __task_cred - Access a task's objective credentials
@@ -307,7 +308,7 @@ static inline void put_cred(const struct cred *_cred)
 ({							\
 	struct user_struct *__u;			\
 	struct cred *__cred;				\
-	__cred = (struct cred *) current_cred();	\
+	__cred = current_cred();			\
 	__u = get_uid(__cred->user);			\
 	__u;						\
 })
@@ -322,7 +323,7 @@ static inline void put_cred(const struct cred *_cred)
 ({							\
 	struct group_info *__groups;			\
 	struct cred *__cred;				\
-	__cred = (struct cred *) current_cred();	\
+	__cred = current_cred();			\
 	__groups = get_group_info(__cred->group_info);	\
 	__groups;					\
 })
@@ -341,7 +342,7 @@ static inline void put_cred(const struct cred *_cred)
 
 #define current_cred_xxx(xxx)			\
 ({						\
-	current->cred->xxx;			\
+	current_cred()->xxx;			\
 })
 
 #define current_uid()		(current_cred_xxx(uid))

Re: fix rcu annotations noise in cred.h

[David Howells]

Al Viro <viro@ZenIV.linux.org.uk> wrote:

>  #define current_cred() \
> -	(current->cred)
> +	(*(__force struct cred **)&current->cred)

No.  You've cast away the const.  Please don't do that.

Paul: isn't there some better way of doing this using an RCU wrapper macro?

	#define current_cred() \
		rcu_dereference_protected(current->cred, 1) 

Perhaps?

David

Re: fix rcu annotations noise in cred.h

[Al Viro]

On Mon, Aug 08, 2011 at 09:18:29AM +0100, David Howells wrote:
> 
> Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> >  #define current_cred() \
> > -	(current->cred)
> > +	(*(__force struct cred **)&current->cred)
> 
> No.  You've cast away the const.  Please don't do that.

So let's bring const back, not a problem...

Re: fix rcu annotations noise in cred.h

[Paul E. McKenney]

On Mon, Aug 08, 2011 at 09:18:29AM +0100, David Howells wrote:
> 
> Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> >  #define current_cred() \
> > -	(current->cred)
> > +	(*(__force struct cred **)&current->cred)
> 
> No.  You've cast away the const.  Please don't do that.
> 
> Paul: isn't there some better way of doing this using an RCU wrapper macro?
> 
> 	#define current_cred() \
> 		rcu_dereference_protected(current->cred, 1) 
> 
> Perhaps?

This is indeed what rcu_dereference_protected() is intended for.

							Thanx, Paul

Re: fix rcu annotations noise in cred.h

[David Howells]

Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> This is indeed what rcu_dereference_protected() is intended for.

How about the attached patch?

David
---
From: David Howells <dhowells@redhat.com>
Subject: [PATCH] CRED: Restore const to current_cred()

Commit 3295514841c2112d94451ba5deaf54f5afb78ea9 accidentally dropped the const
of current->cred inside current_cred() by the insertion of a cast to deal with
an RCU annotation loss warning from sparce.

Use an appropriate RCU wrapper instead so as not to lose the const.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Al Viro <viro@zeniv.linux.org.uk>
---

 include/linux/cred.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/include/linux/cred.h b/include/linux/cred.h
index 98f46ef..8e2fd44 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -269,7 +269,7 @@ static inline void put_cred(const struct cred *_cred)
  * since nobody else can modify it.
  */
 #define current_cred() \
-	(*(__force struct cred **)&current->cred)
+	rcu_dereference_protected(current->cred, 1)
 
 /**
  * __task_cred - Access a task's objective credentials

Re: fix rcu annotations noise in cred.h

[Paul E. McKenney]

On Mon, Aug 08, 2011 at 01:54:08PM +0100, David Howells wrote:
> Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > This is indeed what rcu_dereference_protected() is intended for.
> 
> How about the attached patch?
> 
> David
> ---
> From: David Howells <dhowells@redhat.com>
> Subject: [PATCH] CRED: Restore const to current_cred()
> 
> Commit 3295514841c2112d94451ba5deaf54f5afb78ea9 accidentally dropped the const
> of current->cred inside current_cred() by the insertion of a cast to deal with
> an RCU annotation loss warning from sparce.
> 
> Use an appropriate RCU wrapper instead so as not to lose the const.

Reviewed-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Al Viro <viro@zeniv.linux.org.uk>
> ---
> 
>  include/linux/cred.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/include/linux/cred.h b/include/linux/cred.h
> index 98f46ef..8e2fd44 100644
> --- a/include/linux/cred.h
> +++ b/include/linux/cred.h
> @@ -269,7 +269,7 @@ static inline void put_cred(const struct cred *_cred)
>   * since nobody else can modify it.
>   */
>  #define current_cred() \
> -	(*(__force struct cred **)&current->cred)
> +	rcu_dereference_protected(current->cred, 1)
> 
>  /**
>   * __task_cred - Access a task's objective credentials

Re: fix rcu annotations noise in cred.h

[David Howells]

Al Viro <viro@ZenIV.linux.org.uk> wrote:

> task->cred is declared as __rcu, and access to other tasks' ->cred is,
> indeed, protected.  Access to current->cred does not need rcu_dereference()
> at all, since only the task itself can change its ->cred.  sparse, of
> course, has no way of knowing that...

What was the Sparse warning, btw?  You didn't say.  I don't see any warnings,
but it's possible I have a too-old version of Sparse or the wrong config
options?  (Or maybe I just don't know what to look for)

David