Re: [PATCH] Initial support for struct vfs_cred [0/1]

[Luca Barbieri <ldb@ldb.ods.org>]

[-- Attachment #1: Type: text/plain, Size: 3512 bytes --]

On Sun, 2002-09-01 at 00:30, Trond Myklebust wrote:
> >>>>> " " == Luca Barbieri <ldb@ldb.ods.org> writes:
> 
>      > Then the rest of the code doesn't need to know at all that
>      > credentials are shared and is simpler and faster.  We have
>      > however a larger penalty on credential change but, as you say,
>      > that's extremely rare (well, perhaps not necessarily extremely,
>      > but still rare).
> 
> What if I, in a fit of madness/perversion, decide to use CLONE_CRED
> between 2 kernel threads (i.e. no 'kernel entry')?
You don't or you manually patch the task_struct of the other threads.
This isn't a serious concern.

> Leaving CLONE_CRED aside, please do not forget that most of the
> motivation for vfs_cred is the need to *cache* credentials.
> This is something which we already do today in several filesystems:
> Coda, Intermezzo, NFS, to name but the most obvious.
> The result of the lack of a VFS-sanctioned credential is that we have
> to use 'struct file' as a vehicle for passing credentials in, for
> instance, the address_space_operations, and that each filesystem ends
> up having to keep its own private copies of those credentials in
> file->private_data.
I'm not proposing to not use vfs_cred (the pseudocode I written includes
it).
I'm just suggesting a way to speed up access to credentials.
Your code does get_current_vfscred()/put_vfscred() in normal paths and
accesses credentials with an indirect pointer.
Furthermore, setting the uid and gid are very expensive since they need
to break copy-on-write.
If we instead only allow changes to credentials on kernel mode entry, we
don't have to worry about changing credentials and the code becomes
simpler and faster.
It also seems better to only copy-on-write groups and just copy uid and
gid.


For example, rather than this;
-               uid_t saved_fsuid = current->fsuid;
+               struct vfs_cred *saved_cred = get_current_vfscred();
                kernel_cap_t saved_cap = current->cap_effective;
 
                /* Create RPC socket as root user so we get a priv port
*/
-               current->fsuid = 0;
+               setfsuid(0);
                cap_raise (current->cap_effective,
CAP_NET_BIND_SERVICE);
                xprt = xprt_create_proto(host->h_proto, &host->h_addr,
NULL);
-               current->fsuid = saved_fsuid;
+               set_current_vfscred(saved_cred);
+               put_vfscred(saved_cred);

you can just do this:
-               uid_t saved_fsuid = current->fsuid;
+               uid_t saved_fsuid = current->fscred.uid;
                kernel_cap_t saved_cap = current->cap_effective;
 
                /* Create RPC socket as root user so we get a priv port
*/
-               current->fsuid = 0;
+               current->fscred.uid = 0;
                cap_raise (current->cap_effective,
CAP_NET_BIND_SERVICE);
                xprt = xprt_create_proto(host->h_proto, &host->h_addr,
NULL);
-               current->fsuid = saved_fsuid;
+               current->fscred.uid = saved_fsuid;

(here the fscred is the one called cred in the pseudocode: fscred is a
better name).

This code uses vfs_cred but is as fast as the old one while the one in
your patch is significantly slower (e.g. compare your setfsuid with
current->fscred.uid = xxx).
If you then need to keep the vfs_cred data for longer than a single
syscall, just copy it like you would do in the propagation code.
That means copying the structure and then incrementing the reference
count on groups.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]