* fix rcu annotations noise in cred.h
@ 2011-08-07 17:55 Al Viro
2011-08-08 8:18 ` David Howells
2011-08-08 12:28 ` David Howells
0 siblings, 2 replies; 7+ messages in thread
From: Al Viro @ 2011-08-07 17:55 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, David Howells
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 **)¤t->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))
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: fix rcu annotations noise in cred.h
2011-08-07 17:55 fix rcu annotations noise in cred.h Al Viro
@ 2011-08-08 8:18 ` David Howells
2011-08-08 8:28 ` Al Viro
` (2 more replies)
2011-08-08 12:28 ` David Howells
1 sibling, 3 replies; 7+ messages in thread
From: David Howells @ 2011-08-08 8:18 UTC (permalink / raw)
To: Al Viro, Linus Torvalds, Paul E. McKenney; +Cc: dhowells, linux-kernel
Al Viro <viro@ZenIV.linux.org.uk> wrote:
> #define current_cred() \
> - (current->cred)
> + (*(__force struct cred **)¤t->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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: fix rcu annotations noise in cred.h
2011-08-08 8:18 ` David Howells
@ 2011-08-08 8:28 ` Al Viro
2011-08-08 12:34 ` Paul E. McKenney
2011-08-08 12:54 ` David Howells
2 siblings, 0 replies; 7+ messages in thread
From: Al Viro @ 2011-08-08 8:28 UTC (permalink / raw)
To: David Howells; +Cc: Linus Torvalds, Paul E. McKenney, linux-kernel
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 **)¤t->cred)
>
> No. You've cast away the const. Please don't do that.
So let's bring const back, not a problem...
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: fix rcu annotations noise in cred.h
2011-08-07 17:55 fix rcu annotations noise in cred.h Al Viro
2011-08-08 8:18 ` David Howells
@ 2011-08-08 12:28 ` David Howells
1 sibling, 0 replies; 7+ messages in thread
From: David Howells @ 2011-08-08 12:28 UTC (permalink / raw)
To: Al Viro; +Cc: dhowells, Linus Torvalds, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: fix rcu annotations noise in cred.h
2011-08-08 8:18 ` David Howells
2011-08-08 8:28 ` Al Viro
@ 2011-08-08 12:34 ` Paul E. McKenney
2011-08-08 12:54 ` David Howells
2 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2011-08-08 12:34 UTC (permalink / raw)
To: David Howells; +Cc: Al Viro, Linus Torvalds, linux-kernel
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 **)¤t->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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: fix rcu annotations noise in cred.h
2011-08-08 8:18 ` David Howells
2011-08-08 8:28 ` Al Viro
2011-08-08 12:34 ` Paul E. McKenney
@ 2011-08-08 12:54 ` David Howells
2011-08-08 14:35 ` Paul E. McKenney
2 siblings, 1 reply; 7+ messages in thread
From: David Howells @ 2011-08-08 12:54 UTC (permalink / raw)
To: paulmck; +Cc: dhowells, Al Viro, Linus Torvalds, linux-kernel
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 **)¤t->cred)
+ rcu_dereference_protected(current->cred, 1)
/**
* __task_cred - Access a task's objective credentials
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: fix rcu annotations noise in cred.h
2011-08-08 12:54 ` David Howells
@ 2011-08-08 14:35 ` Paul E. McKenney
0 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2011-08-08 14:35 UTC (permalink / raw)
To: David Howells; +Cc: Al Viro, Linus Torvalds, linux-kernel
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 **)¤t->cred)
> + rcu_dereference_protected(current->cred, 1)
>
> /**
> * __task_cred - Access a task's objective credentials
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-08-08 14:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-07 17:55 fix rcu annotations noise in cred.h Al Viro
2011-08-08 8:18 ` David Howells
2011-08-08 8:28 ` Al Viro
2011-08-08 12:34 ` Paul E. McKenney
2011-08-08 12:54 ` David Howells
2011-08-08 14:35 ` Paul E. McKenney
2011-08-08 12:28 ` David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).