On Sep. 14, 2009, 19:49 +0300, "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> On Mon, Sep 14, 2009 at 12:35:35PM -0400, bfields wrote:
>> On Thu, Sep 10, 2009 at 12:25:59PM +0300, Benny Halevy wrote:
>>> From: Andy Adamson <andros@xxxxxxxxxx>
>>>
>>> Keep the xprt used for create_session in cl_cb_xprt.
>>> Mark cl_callback.cb_minorversion = 1 and remember
>>> the client provided cl_callback.cb_prog rpc program number.
>>> Use it to probe the callback path.
>>>
>>> Use the client's network address to initialize as the
>>> callback's address as expected by the xprt creation
>>> routines.
>>>
>>> Define xdr sizes and code nfs4_cb_compound header to be able
>>> to send a null callback rpc.
>>>
>>> Signed-off-by: Andy Adamson<andros@xxxxxxxxxx>
>>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>>> Signed-off-by: Ricardo Labiaga <Ricardo.Labiaga@xxxxxxxxxx>
>>> [get callback minorversion from fore channel's]
>>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>>> [nfsd41: change bc_sock to bc_xprt]
>>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>>> [pulled definition for cl_cb_xprt]
>>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>>> [nfsd41: set up backchannel's cb_addr]
>>> [moved rpc_create_args init to "nfsd: modify nfsd4.1 backchannel to use new xprt class"]
>>> Signed-off-by: Benny Halevy <bhalevy@xxxxxxxxxxx>
>>> ---
>>> fs/nfsd/nfs4callback.c | 21 +++++++++++++++++++--
>>> fs/nfsd/nfs4state.c | 14 ++++++++++++++
>>> include/linux/nfsd/state.h | 3 +++
>>> 3 files changed, 36 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
>>> index 63bb384..3e3e15b 100644
>>> --- a/fs/nfsd/nfs4callback.c
>>> +++ b/fs/nfsd/nfs4callback.c
>>> @@ -43,6 +43,7 @@
>>> #include <linux/sunrpc/xdr.h>
>>> #include <linux/sunrpc/svc.h>
>>> #include <linux/sunrpc/clnt.h>
>>> +#include <linux/sunrpc/svcsock.h>
>>> #include <linux/nfsd/nfsd.h>
>>> #include <linux/nfsd/state.h>
>>> #include <linux/sunrpc/sched.h>
>>> @@ -52,16 +53,19 @@
>>>
>>> #define NFSPROC4_CB_NULL 0
>>> #define NFSPROC4_CB_COMPOUND 1
>>> +#define NFS4_STATEID_SIZE 16
>>>
>>> /* Index of predefined Linux callback client operations */
>>>
>>> enum {
>>> NFSPROC4_CLNT_CB_NULL = 0,
>>> NFSPROC4_CLNT_CB_RECALL,
>>> + NFSPROC4_CLNT_CB_SEQUENCE,
>>> };
>>>
>>> enum nfs_cb_opnum4 {
>>> OP_CB_RECALL = 4,
>>> + OP_CB_SEQUENCE = 11,
>>> };
>>>
>>> #define NFS4_MAXTAGLEN 20
>>> @@ -70,15 +74,22 @@ enum nfs_cb_opnum4 {
>>> #define NFS4_dec_cb_null_sz 0
>>> #define cb_compound_enc_hdr_sz 4
>>> #define cb_compound_dec_hdr_sz (3 + (NFS4_MAXTAGLEN >> 2))
>>> +#define sessionid_sz (NFS4_MAX_SESSIONID_LEN >> 2)
>>> +#define cb_sequence_enc_sz (sessionid_sz + 4 + \
>>> + 1 /* no referring calls list yet */)
>>> +#define cb_sequence_dec_sz (op_dec_sz + sessionid_sz + 4)
>>> +
>>> #define op_enc_sz 1
>>> #define op_dec_sz 2
>>> #define enc_nfs4_fh_sz (1 + (NFS4_FHSIZE >> 2))
>>> #define enc_stateid_sz (NFS4_STATEID_SIZE >> 2)
>>> #define NFS4_enc_cb_recall_sz (cb_compound_enc_hdr_sz + \
>>> + cb_sequence_enc_sz + \
>>> 1 + enc_stateid_sz + \
>>> enc_nfs4_fh_sz)
>>>
>>> #define NFS4_dec_cb_recall_sz (cb_compound_dec_hdr_sz + \
>>> + cb_sequence_dec_sz + \
>>> op_dec_sz)
>>>
>>> /*
>>> @@ -137,11 +148,13 @@ xdr_error: \
>>> } while (0)
>>>
>>> struct nfs4_cb_compound_hdr {
>>> - int status;
>>> - u32 ident;
>>> + /* args */
>>> + u32 ident; /* minorversion 0 only */
>>> u32 nops;
>>> __be32 *nops_p;
>>> u32 minorversion;
>>> + /* res */
>>> + int status;
>>> u32 taglen;
>>> char *tag;
>>> };
>>> @@ -399,6 +412,10 @@ int setup_callback_client(struct nfs4_client *clp)
>>> if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
>>> return -EINVAL;
>>>
>>> + dprintk("%s: program %s 0x%x nrvers %u version %u minorversion %u\n",
>>> + __func__, args.program->name, args.prognumber,
>>> + args.program->nrvers, args.version, cb->cb_minorversion);
>>> +
>>> /* Create RPC client */
>>> client = rpc_create(&args);
>>> if (IS_ERR(client)) {
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index 46e9ac5..e4c3223 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -706,6 +706,8 @@ static inline void
>>> free_client(struct nfs4_client *clp)
>>> {
>>> shutdown_callback_client(clp);
>>> + if (clp->cl_cb_xprt)
>>> + svc_xprt_put(clp->cl_cb_xprt);
>>> if (clp->cl_cred.cr_group_info)
>>> put_group_info(clp->cl_cred.cr_group_info);
>>> kfree(clp->cl_principal);
>>> @@ -1321,6 +1323,18 @@ nfsd4_create_session(struct svc_rqst *rqstp,
>>> cr_ses->flags &= ~SESSION4_PERSIST;
>>> cr_ses->flags &= ~SESSION4_RDMA;
>>>
>>> + if (cr_ses->flags & SESSION4_BACK_CHAN) {
>>> + unconf->cl_cb_xprt = rqstp->rq_xprt;
>>> + svc_xprt_get(unconf->cl_cb_xprt);
>>> + rpc_copy_addr(
>>> + (struct sockaddr *)&unconf->cl_cb_conn.cb_addr,
>>> + sa);
>>> + unconf->cl_cb_conn.cb_addrlen = svc_addr_len(sa);
>>> + unconf->cl_cb_conn.cb_minorversion =
>>> + cstate->minorversion;
>>> + unconf->cl_cb_conn.cb_prog = cr_ses->callback_prog;
>>> + nfsd4_probe_callback(unconf);
>> This results in a NULL deference in rpcauth_lookup_credcache()--probably
>> some callback parameters that aren't set up right yet.
Where exactly is the NULL deref?
>
> Note--that's fixed 7 patches later in fsd41: Refactor create_client(),
> but I don't actually understand how yet.
unconf's cl_flavor initialization was moved in the latter patch
from nfsd4_setclientid to create_client so maybe this could
be the culprit (though, assuming it is initialized to 0
it will choosing implicitly authnull_ops in rpcauth_create()
which _should_ work...)
Benny
>
> --b.
>
>> --b.
>>
>>> + }
>>> conf = unconf;
>>> } else {
>>> status = nfserr_stale_clientid;
>>> diff --git a/include/linux/nfsd/state.h b/include/linux/nfsd/state.h
>>> index 70ef5f4..243277b 100644
>>> --- a/include/linux/nfsd/state.h
>>> +++ b/include/linux/nfsd/state.h
>>> @@ -212,6 +212,9 @@ struct nfs4_client {
>>> struct nfsd4_clid_slot cl_cs_slot; /* create_session slot */
>>> u32 cl_exchange_flags;
>>> struct nfs4_sessionid cl_sessionid;
>>> +
>>> + /* for nfs41 callbacks */
>>> + struct svc_xprt *cl_cb_xprt; /* 4.1 callback transport */
>>> };
>>>
>>> /* struct nfs4_client_reset
>>> --
>>> 1.6.4
>>>
--
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
