[PATCH 0/2] Introduce credentials API

[PATCH 0/2] Introduce credentials API

[David Howells]

Hi James, Andrew,

Here are the patches that are necessary just to add the credentials wrapper
API as Stephen suggested.

There are two patches: the first renames some things in XFS that would
otherwise collide in the namespace, and the second adds the wrappers in no-op
forms.

David

[PATCH 1/2] CRED: Alter XFS so as to avoid namespace collisions with upcoming COW creds

[David Howells]

Rename XFS's current_fsuid() and current_fsgid() to avoid collision with
functions of the same name in upcoming COW creds.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/xfs/linux-2.6/xfs_linux.h |    4 ++--
 fs/xfs/xfs_inode.c           |    4 ++--
 fs/xfs/xfs_vnodeops.c        |    8 ++++----
 3 files changed, 8 insertions(+), 8 deletions(-)


diff --git a/fs/xfs/linux-2.6/xfs_linux.h b/fs/xfs/linux-2.6/xfs_linux.h
index 4d45d93..05a2e7e 100644
--- a/fs/xfs/linux-2.6/xfs_linux.h
+++ b/fs/xfs/linux-2.6/xfs_linux.h
@@ -126,8 +126,8 @@
 
 #define current_cpu()		(raw_smp_processor_id())
 #define current_pid()		(current->pid)
-#define current_fsuid(cred)	(current->fsuid)
-#define current_fsgid(cred)	(current->fsgid)
+#define this_fsuid(cred)	(current->fsuid)
+#define this_fsgid(cred)	(current->fsgid)
 #define current_test_flags(f)	(current->flags & (f))
 #define current_set_flags_nested(sp, f)		\
 		(*(sp) = current->flags, current->flags |= (f))
diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index bedc661..5e481ae 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1082,8 +1082,8 @@ xfs_ialloc(
 	ip->i_d.di_onlink = 0;
 	ip->i_d.di_nlink = nlink;
 	ASSERT(ip->i_d.di_nlink == nlink);
-	ip->i_d.di_uid = current_fsuid(cr);
-	ip->i_d.di_gid = current_fsgid(cr);
+	ip->i_d.di_uid = this_fsuid(cr);
+	ip->i_d.di_gid = this_fsgid(cr);
 	ip->i_d.di_projid = prid;
 	memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
 
diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
index 76a1166..3dfe8a0 100644
--- a/fs/xfs/xfs_vnodeops.c
+++ b/fs/xfs/xfs_vnodeops.c
@@ -182,7 +182,7 @@ xfs_setattr(
 	xfs_ilock(ip, lock_flags);
 
 	/* boolean: are we the file owner? */
-	file_owner = (current_fsuid(credp) == ip->i_d.di_uid);
+	file_owner = (this_fsuid(credp) == ip->i_d.di_uid);
 
 	/*
 	 * Change various properties of a file.
@@ -1536,7 +1536,7 @@ xfs_create(
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
 	error = XFS_QM_DQVOPALLOC(mp, dp,
-			current_fsuid(credp), current_fsgid(credp), prid,
+			this_fsuid(credp), this_fsgid(credp), prid,
 			XFS_QMOPT_QUOTALL|XFS_QMOPT_INHERIT, &udqp, &gdqp);
 	if (error)
 		goto std_return;
@@ -2352,7 +2352,7 @@ xfs_mkdir(
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
 	error = XFS_QM_DQVOPALLOC(mp, dp,
-			current_fsuid(credp), current_fsgid(credp), prid,
+			this_fsuid(credp), this_fsgid(credp), prid,
 			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
 	if (error)
 		goto std_return;
@@ -2578,7 +2578,7 @@ xfs_symlink(
 	 * Make sure that we have allocated dquot(s) on disk.
 	 */
 	error = XFS_QM_DQVOPALLOC(mp, dp,
-			current_fsuid(credp), current_fsgid(credp), prid,
+			this_fsuid(credp), this_fsgid(credp), prid,
 			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
 	if (error)
 		goto std_return;

Re: [PATCH 1/2] CRED: Alter XFS so as to avoid namespace collisions with upcoming COW creds

[James Morris]

On Tue, 12 Aug 2008, David Howells wrote:

> Rename XFS's current_fsuid() and current_fsgid() to avoid collision with
> functions of the same name in upcoming COW creds.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

Can we get an acked-by from an XFS maintainer before pushing this to 
Linus?

> ---
> 
>  fs/xfs/linux-2.6/xfs_linux.h |    4 ++--
>  fs/xfs/xfs_inode.c           |    4 ++--
>  fs/xfs/xfs_vnodeops.c        |    8 ++++----
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/fs/xfs/linux-2.6/xfs_linux.h b/fs/xfs/linux-2.6/xfs_linux.h
> index 4d45d93..05a2e7e 100644
> --- a/fs/xfs/linux-2.6/xfs_linux.h
> +++ b/fs/xfs/linux-2.6/xfs_linux.h
> @@ -126,8 +126,8 @@
>  
>  #define current_cpu()		(raw_smp_processor_id())
>  #define current_pid()		(current->pid)
> -#define current_fsuid(cred)	(current->fsuid)
> -#define current_fsgid(cred)	(current->fsgid)
> +#define this_fsuid(cred)	(current->fsuid)
> +#define this_fsgid(cred)	(current->fsgid)
>  #define current_test_flags(f)	(current->flags & (f))
>  #define current_set_flags_nested(sp, f)		\
>  		(*(sp) = current->flags, current->flags |= (f))
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index bedc661..5e481ae 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -1082,8 +1082,8 @@ xfs_ialloc(
>  	ip->i_d.di_onlink = 0;
>  	ip->i_d.di_nlink = nlink;
>  	ASSERT(ip->i_d.di_nlink == nlink);
> -	ip->i_d.di_uid = current_fsuid(cr);
> -	ip->i_d.di_gid = current_fsgid(cr);
> +	ip->i_d.di_uid = this_fsuid(cr);
> +	ip->i_d.di_gid = this_fsgid(cr);
>  	ip->i_d.di_projid = prid;
>  	memset(&(ip->i_d.di_pad[0]), 0, sizeof(ip->i_d.di_pad));
>  
> diff --git a/fs/xfs/xfs_vnodeops.c b/fs/xfs/xfs_vnodeops.c
> index 76a1166..3dfe8a0 100644
> --- a/fs/xfs/xfs_vnodeops.c
> +++ b/fs/xfs/xfs_vnodeops.c
> @@ -182,7 +182,7 @@ xfs_setattr(
>  	xfs_ilock(ip, lock_flags);
>  
>  	/* boolean: are we the file owner? */
> -	file_owner = (current_fsuid(credp) == ip->i_d.di_uid);
> +	file_owner = (this_fsuid(credp) == ip->i_d.di_uid);
>  
>  	/*
>  	 * Change various properties of a file.
> @@ -1536,7 +1536,7 @@ xfs_create(
>  	 * Make sure that we have allocated dquot(s) on disk.
>  	 */
>  	error = XFS_QM_DQVOPALLOC(mp, dp,
> -			current_fsuid(credp), current_fsgid(credp), prid,
> +			this_fsuid(credp), this_fsgid(credp), prid,
>  			XFS_QMOPT_QUOTALL|XFS_QMOPT_INHERIT, &udqp, &gdqp);
>  	if (error)
>  		goto std_return;
> @@ -2352,7 +2352,7 @@ xfs_mkdir(
>  	 * Make sure that we have allocated dquot(s) on disk.
>  	 */
>  	error = XFS_QM_DQVOPALLOC(mp, dp,
> -			current_fsuid(credp), current_fsgid(credp), prid,
> +			this_fsuid(credp), this_fsgid(credp), prid,
>  			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
>  	if (error)
>  		goto std_return;
> @@ -2578,7 +2578,7 @@ xfs_symlink(
>  	 * Make sure that we have allocated dquot(s) on disk.
>  	 */
>  	error = XFS_QM_DQVOPALLOC(mp, dp,
> -			current_fsuid(credp), current_fsgid(credp), prid,
> +			this_fsuid(credp), this_fsgid(credp), prid,
>  			XFS_QMOPT_QUOTALL | XFS_QMOPT_INHERIT, &udqp, &gdqp);
>  	if (error)
>  		goto std_return;
> 

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 1/2] CRED: Alter XFS so as to avoid namespace collisions with upcoming COW creds

[Christoph Hellwig]

On Tue, Aug 12, 2008 at 02:28:45PM +0100, David Howells wrote:
> Rename XFS's current_fsuid() and current_fsgid() to avoid collision with
> functions of the same name in upcoming COW creds.

Just switch it directly to your new version without the paramter.

[PATCH 2/2] CRED: Introduce credential access wrappers

[David Howells]

The patches that are intended to introduce copy-on-write credentials for 2.6.28
require abstraction of access to some fields of the task structure,
particularly for the case of one task accessing another's credentials where RCU
will have to be observed.

Introduced here are trivial no-op versions of the desired accessors for current
and other tasks so that other subsystems can start to be converted over more
easily.

Wrappers are introduced into a new header (linux/cred.h) for UID/GID,
EUID/EGID, SUID/SGID, FSUID/FSGID, cap_effective and current's subscribed
user_struct.

linux/cred.h is #included from linux/sched.h.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/linux/cred.h  |   50 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/sched.h |    1 +
 2 files changed, 51 insertions(+), 0 deletions(-)
 create mode 100644 include/linux/cred.h


diff --git a/include/linux/cred.h b/include/linux/cred.h
new file mode 100644
index 0000000..b69222c
--- /dev/null
+++ b/include/linux/cred.h
@@ -0,0 +1,50 @@
+/* Credentials management
+ *
+ * Copyright (C) 2008 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#ifndef _LINUX_CRED_H
+#define _LINUX_CRED_H
+
+#define get_current_user()	(get_uid(current->user))
+
+#define task_uid(task)		((task)->uid)
+#define task_gid(task)		((task)->gid)
+#define task_euid(task)		((task)->euid)
+#define task_egid(task)		((task)->egid)
+
+#define current_uid()		(current->uid)
+#define current_gid()		(current->gid)
+#define current_euid()		(current->euid)
+#define current_egid()		(current->egid)
+#define current_suid()		(current->suid)
+#define current_sgid()		(current->sgid)
+#define current_fsuid()		(current->fsuid)
+#define current_fsgid()		(current->fsgid)
+#define current_cap()		(current->cap_effective)
+
+#define current_uid_gid(_uid, _gid)		\
+do {						\
+	*(_uid) = current->uid;			\
+	*(_gid) = current->gid;			\
+} while(0)
+
+#define current_euid_egid(_uid, _gid)		\
+do {						\
+	*(_uid) = current->euid;		\
+	*(_gid) = current->egid;		\
+} while(0)
+
+#define current_fsuid_fsgid(_uid, _gid)		\
+do {						\
+	*(_uid) = current->fsuid;		\
+	*(_gid) = current->fsgid;		\
+} while(0)
+
+#endif /* _LINUX_CRED_H */
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 5270d44..9fd5694 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -87,6 +87,7 @@ struct sched_param {
 #include <linux/task_io_accounting.h>
 #include <linux/kobject.h>
 #include <linux/latencytop.h>
+#include <linux/cred.h>
 
 #include <asm/processor.h>
 

Re: [PATCH 2/2] CRED: Introduce credential access wrappers

[Ingo Oeser]

Hi David,

On Tuesday 12 August 2008, David Howells wrote:
> Wrappers are introduced into a new header (linux/cred.h) for UID/GID,
> EUID/EGID, SUID/SGID, FSUID/FSGID, cap_effective and current's subscribed
> user_struct.

Why macros? When introducing APIs using trivial inlines makes sure that 
the conversion is correct, type correct and side effect free for the callers.
Macros cannot ensure this without pain.

Please respin this patch with inlines. If that needs header changes,
your CRED subsytem might need that, too.


Best Regards

Ingo Oeser

Re: [PATCH 2/2] CRED: Introduce credential access wrappers

[David Howells]

Ingo Oeser <ioe-lkml@rameria.de> wrote:

> Why macros? When introducing APIs using trivial inlines makes sure that 
> the conversion is correct, type correct and side effect free for the callers.
> Macros cannot ensure this without pain.

But macros don't care that you can't access current at this point.

David

Re: [PATCH 2/2] CRED: Introduce credential access wrappers

[Ingo Oeser]

Hi David,

On Tuesday 12 August 2008, David Howells wrote:
> Ingo Oeser <ioe-lkml@rameria.de> wrote:
> 
> > Why macros? When introducing APIs using trivial inlines makes sure that 
> > the conversion is correct, type correct and side effect free for the callers.
> > Macros cannot ensure this without pain.
> 
> But macros don't care that you can't access current at this point.

Ok, if that is the only reason, please mention this in the commit message.
That would be enough for me to not wonder about this issue.

But I wonder how you solve this issue if the amount of code behinde your wrappers
grows.

If you have to take them out of line, it would give me a good indication
of the costs involved in the later bodies of these now trivial wrappers,
if you do it right away. 

That way you'll find missing includes and whatnot while converting existing code
to the trivial wrappers.


Best Regards

Ingo Oeser