On 01/18/2018 01:39 PM, Anna Schumaker wrote:
> Hi Neil,
>
> On 01/08/2018 12:26 AM, NeilBrown wrote:
>> The SUNRPC credential framework was put together before
>> Linux has 'struct cred'. Now that we have it, it makes sense to
>> use it.
>> This first step just includes a suitable 'struct cred *' pointer
>> in every 'struct auth_cred' and almost every 'struct rpc_cred'.
>>
>> The rpc_cred used for auth_null has a NULL 'struct cred *' as nothing
>> else really makes sense.
>>
>> For rpc_cred, the pointer is reference counted.
>> For auth_cred it isn't. struct auth_cred are either allocated on
>> the stack, in which case the thread owns a reference to the auth,
>> or are part of 'struct generic_cred' in which case gc_base owns the
>> reference and acred shares it.
This patch is also causing a kernel panic for me if I mount using sec=krb5, run cthon tests, and then unmount. Here is the log message I'm getting:
[ 82.599174] Kernel panic - not syncing: CRED: put_cred_rcu() sees 00000000f5847a57 with usage -1
[ 82.599174]
[ 82.600227] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.15.0-rc7-ANNA+ #14336
[ 82.600801] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[ 82.601435] Call Trace:
[ 82.601639] <IRQ>
[ 82.601830] dump_stack+0x5c/0x7e
[ 82.602125] panic+0xdf/0x228
[ 82.602383] ? try_to_wake_up+0x24b/0x420
[ 82.602853] put_cred_rcu+0x8a/0x90
[ 82.603183] rcu_process_callbacks+0x1ab/0x4f0
[ 82.603577] __do_softirq+0xcc/0x305
[ 82.603881] irq_exit+0xa9/0xb0
[ 82.604159] smp_apic_timer_interrupt+0x5b/0x140
[ 82.604528] apic_timer_interrupt+0x98/0xa0
[ 82.604892] </IRQ>
[ 82.605133] RIP: 0010:native_safe_halt+0x2/0x10
[ 82.605678] RSP: 0018:ffffffff82003ea8 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff11
[ 82.606619] RAX: 0000000080000000 RBX: 0000000000000000 RCX: 0000000000000000
[ 82.607270] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 82.608077] RBP: 0000000000000000 R08: 0000000000000002 R09: 000000000001ea40
[ 82.609066] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000000
[ 82.609951] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 82.610771] default_idle+0x15/0x120
[ 82.611128] do_idle+0x15c/0x1c0
[ 82.611371] cpu_startup_entry+0x6a/0x70
[ 82.611762] start_kernel+0x445/0x465
[ 82.612104] secondary_startup_64+0xa5/0xb0
[ 82.612561] Kernel Offset: disabled
[ 82.612862] ---[ end Kernel panic - not syncing: CRED: put_cred_rcu() sees 00000000f5847a57 with usage -1
[ 82.612862]
Thanks,
Anna
>>
>> Signed-off-by: NeilBrown <neilb@xxxxxxxx>
>> ---
>> fs/nfs/flexfilelayout/flexfilelayout.c | 17 +++++++++++++++++
>> fs/nfsd/nfs4callback.c | 13 ++++++++++++-
>> include/linux/sunrpc/auth.h | 2 ++
>> net/sunrpc/auth.c | 15 +++++++++++++--
>> net/sunrpc/auth_generic.c | 7 ++++++-
>> net/sunrpc/auth_gss/auth_gss.c | 1 +
>> 6 files changed, 51 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
>> index c75ad982bcfc..b727579a1508 100644
>> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
>> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
>> @@ -9,6 +9,7 @@
>> #include <linux/nfs_fs.h>
>> #include <linux/nfs_page.h>
>> #include <linux/module.h>
>> +#include <linux/sched/mm.h>
>>
>> #include <linux/sunrpc/metrics.h>
>>
>> @@ -415,6 +416,7 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh,
>> struct nfs4_ff_layout_mirror *mirror;
>> struct auth_cred acred = { .group_info = ff_zero_group };
>> struct rpc_cred __rcu *cred;
>> + struct cred *kcred;
>> u32 ds_count, fh_count, id;
>> int j;
>>
>> @@ -491,8 +493,23 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh,
>>
>> acred.gid = make_kgid(&init_user_ns, id);
>>
>> + if (gfp_flags & __GFP_FS)
>> + kcred = prepare_kernel_cred(NULL);
>> + else {
>> + unsigned int nofs_flags = memalloc_nofs_save();
>> + kcred = prepare_kernel_cred(NULL);
>> + memalloc_nofs_restore(nofs_flags);
>> + }
>> + rc = -ENOMEM;
>> + if (!kcred)
>> + goto out_err_free;
>> + kcred->fsuid = acred.uid;
>> + kcred->fsgid = acred.gid;
>> + acred.cred = kcred;
>> +
>> /* find the cred for it */
>> rcu_assign_pointer(cred, rpc_lookup_generic_cred(&acred, 0, gfp_flags));
>> + put_cred(kcred);
>> if (IS_ERR(cred)) {
>> rc = PTR_ERR(cred);
>> goto out_err_free;
>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
>> index 49b0a9e7ff18..fc5b38ee6c70 100644
>> --- a/fs/nfsd/nfs4callback.c
>> +++ b/fs/nfsd/nfs4callback.c
>> @@ -773,10 +773,21 @@ static struct rpc_cred *get_backchannel_cred(struct nfs4_client *clp, struct rpc
>> } else {
>> struct rpc_auth *auth = client->cl_auth;
>> struct auth_cred acred = {};
>> + struct cred *kcred;
>> + struct rpc_cred *ret;
>> +
>> + kcred = prepare_kernel_cred(NULL);
>> + if (!acred.cred)
>> + return NULL;
>>
>> acred.uid = ses->se_cb_sec.uid;
>> acred.gid = ses->se_cb_sec.gid;
>> - return auth->au_ops->lookup_cred(client->cl_auth, &acred, 0);
>> + kcred->uid = acred.uid;
>> + kcred->gid = acred.gid;
>> + acred.cred = kcred;
>> + ret = auth->au_ops->lookup_cred(client->cl_auth, &acred, 0);
>> + put_cred(kcred);
>> + return ret;
>> }
>> }
>>
>> diff --git a/include/linux/sunrpc/auth.h b/include/linux/sunrpc/auth.h
>> index d9af474a857d..57633e241d4a 100644
>> --- a/include/linux/sunrpc/auth.h
>> +++ b/include/linux/sunrpc/auth.h
>> @@ -46,6 +46,7 @@ enum {
>>
>> /* Work around the lack of a VFS credential */
>> struct auth_cred {
>> + const struct cred *cred;
>> kuid_t uid;
>> kgid_t gid;
>> struct group_info *group_info;
>> @@ -68,6 +69,7 @@ struct rpc_cred {
>> unsigned long cr_expire; /* when to gc */
>> unsigned long cr_flags; /* various flags */
>> atomic_t cr_count; /* ref count */
>> + const struct cred *cr_cred;
>>
>> kuid_t cr_uid;
>>
>> diff --git a/net/sunrpc/auth.c b/net/sunrpc/auth.c
>> index d2623b9f23d6..fd9635dbc17f 100644
>> --- a/net/sunrpc/auth.c
>> +++ b/net/sunrpc/auth.c
>> @@ -634,6 +634,7 @@ rpcauth_lookupcred(struct rpc_auth *auth, int flags)
>> acred.uid = cred->fsuid;
>> acred.gid = cred->fsgid;
>> acred.group_info = cred->group_info;
>> + acred.cred = cred;
>> ret = auth->au_ops->lookup_cred(auth, &acred, flags);
>> return ret;
>> }
>> @@ -649,6 +650,7 @@ rpcauth_init_cred(struct rpc_cred *cred, const struct auth_cred *acred,
>> cred->cr_auth = auth;
>> cred->cr_ops = ops;
>> cred->cr_expire = jiffies;
>> + cred->cr_cred = get_cred(acred->cred);
>> cred->cr_uid = acred->uid;
>> }
>> EXPORT_SYMBOL_GPL(rpcauth_init_cred);
>> @@ -669,11 +671,15 @@ rpcauth_bind_root_cred(struct rpc_task *task, int lookupflags)
>> struct auth_cred acred = {
>> .uid = GLOBAL_ROOT_UID,
>> .gid = GLOBAL_ROOT_GID,
>> + .cred = get_task_cred(&init_task),
>
> Is there a patch somewhere to add "EXPORT_SYMBOL_GPL(get_task_cred)" to kernel/cred.c?
> I'm getting:
> ERROR: "get_task_cred" [net/sunrpc/sunrpc.ko] undefined!
> when I compile.
>
> Thanks,
> Anna
>
>> };
>> + struct rpc_cred *ret;
>>
>> dprintk("RPC: %5u looking up %s cred\n",
>> task->tk_pid, task->tk_client->cl_auth->au_ops->au_name);
>> - return auth->au_ops->lookup_cred(auth, &acred, lookupflags);
>> + ret = auth->au_ops->lookup_cred(auth, &acred, lookupflags);
>> + put_cred(acred.cred);
>> + return ret;
>> }
>>
>> static struct rpc_cred *
>> @@ -715,8 +721,11 @@ put_rpccred(struct rpc_cred *cred)
>> return;
>> /* Fast path for unhashed credentials */
>> if (test_bit(RPCAUTH_CRED_HASHED, &cred->cr_flags) == 0) {
>> - if (atomic_dec_and_test(&cred->cr_count))
>> + if (atomic_dec_and_test(&cred->cr_count)) {
>> + if (cred->cr_cred)
>> + put_cred(cred->cr_cred);
>> cred->cr_ops->crdestroy(cred);
>> + }
>> return;
>> }
>>
>> @@ -739,6 +748,8 @@ put_rpccred(struct rpc_cred *cred)
>> }
>> }
>> spin_unlock(&rpc_credcache_lock);
>> + if (cred->cr_cred)
>> + put_cred(cred->cr_cred);
>> cred->cr_ops->crdestroy(cred);
>> return;
>> out_nodestroy:
>> diff --git a/net/sunrpc/auth_generic.c b/net/sunrpc/auth_generic.c
>> index f1df9837f1ac..08bc5fac1865 100644
>> --- a/net/sunrpc/auth_generic.c
>> +++ b/net/sunrpc/auth_generic.c
>> @@ -61,11 +61,15 @@ struct rpc_cred *rpc_lookup_machine_cred(const char *service_name)
>> .gid = RPC_MACHINE_CRED_GROUPID,
>> .principal = service_name,
>> .machine_cred = 1,
>> + .cred = get_task_cred(&init_task),
>> };
>> + struct rpc_cred *ret;
>>
>> dprintk("RPC: looking up machine cred for service %s\n",
>> service_name);
>> - return generic_auth.au_ops->lookup_cred(&generic_auth, &acred, 0);
>> + ret = generic_auth.au_ops->lookup_cred(&generic_auth, &acred, 0);
>> + put_cred(acred.cred);
>> + return ret;
>> }
>> EXPORT_SYMBOL_GPL(rpc_lookup_machine_cred);
>>
>> @@ -110,6 +114,7 @@ generic_create_cred(struct rpc_auth *auth, struct auth_cred *acred, int flags, g
>> gcred->acred.uid = acred->uid;
>> gcred->acred.gid = acred->gid;
>> gcred->acred.group_info = acred->group_info;
>> + gcred->acred.cred = gcred->gc_base.cr_cred;
>> gcred->acred.ac_flags = 0;
>> if (gcred->acred.group_info != NULL)
>> get_group_info(gcred->acred.group_info);
>> diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
>> index 9463af4b32e8..82301105b4f6 100644
>> --- a/net/sunrpc/auth_gss/auth_gss.c
>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>> @@ -1553,6 +1553,7 @@ static int gss_renew_cred(struct rpc_task *task)
>> struct rpc_auth *auth = oldcred->cr_auth;
>> struct auth_cred acred = {
>> .uid = oldcred->cr_uid,
>> + .cred = oldcred->cr_cred,
>> .principal = gss_cred->gc_principal,
>> .machine_cred = (gss_cred->gc_principal != NULL ? 1 : 0),
>> };
>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
