date:20161010

Re: [Nfs-ganesha-devel] [ntirpc] refcount issue ?

2016-10-10 Thread Matt W. Benjamin

Hi Swen,

Yes, please do.

Matt

- "Swen Schillig"  wrote:

> On Mo, 2016-10-10 at 11:46 -0400, Matt Benjamin wrote:
> > Hi Swen,
> >
> > Are you going to re-push your recent closed PR as 2 new PRs as
> > planned?
> >
> > Dan has made some time this week to review and test, so if you have
> > time, it will be easier to get to merge.
> >
> > Cheers,
> >
> > Matt
> Hi Matt
> 
> It is re-pushed , I just didn't create a new PR yet.
> I was thinking we should test and review before creating a new PR.
> 
> But if you prefer to have a PR right away, I can do that.
> 
> Cheers Swen
> >
> > - Original Message -
> > >
> > > From: "Matt Benjamin" 
> > > To: "Swen Schillig" 
> > > Cc: "Daniel Gryniewicz" , "nfs-ganesha-devel"
>  > > -ganesha-de...@lists.sourceforge.net>
> > > Sent: Tuesday, September 6, 2016 4:16:39 PM
> > > Subject: Re: [ntirpc] refcount issue ?
> > >
> > > Hi,
> > >
> > > inline
> > >
> > > - Original Message -
> > > >
> > > > From: "Swen Schillig" 
> > > > To: "Matt Benjamin" , "Daniel Gryniewicz"
> > > > 
> > > > Cc: "nfs-ganesha-devel"
> 
> > > > Sent: Tuesday, September 6, 2016 4:10:32 PM
> > > > Subject: [ntirpc] refcount issue ?
> > > >
> > > > Matt, Dan.
> > > >
> > > > Could you please have a look at the following code areas and
> > > > verify
> > > > what I think is a refcount issue.
> > > >
> > > > clnt_vc_ncreate2()
> > > > {
> > > > ...
> > > > if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) {
> > > > xd = rec->hdl.xd = alloc_x_vc_data();
> > > > ...
> > > > } else {
> > > > xd = rec->hdl.xd;
> > > > ++(xd->refcnt);     <=== this is not right. we're not
> > > > taking an addtl'
> > > > ref
> > >
> > > Aren't we?  We now share a ref to previously allocated
> rec->hdl.xd.
> > >
> > > >
> > > > here.
> > > > }
> > > > ...
> > > > clnt->cl_p1 = xd;           <=== but here we should increment
> > > > the
> > > > refocunt.
> > > > }
> > > >
> > > > another code section with the same handling.
> > > >
> > > > makefd_xprt()
> > > > {
> > > > ...
> > > > if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) {
> > > > newxd = true;
> > > > xd = rec->hdl.xd = alloc_x_vc_data();
> > > > ...
> > > > } else {
> > > > xd = (struct x_vc_data *)rec->hdl.xd;
> > > > /* dont return destroyed xprts */
> > > > if (!(xd->flags & X_VC_DATA_FLAG_SVC_DESTROYED)) {
> > > > if (rec->hdl.xprt) {
> > > > xprt = rec->hdl.xprt;
> > > > /* inc xprt refcnt */
> > > > SVC_REF(xprt, SVC_REF_FLAG_NONE);
> > > > } else
> > > > ++(xd->refcnt);    < not right, no
> > > > addtl' ref to xd taken.
> > >
> > > so this looks more likely to be incorrect, need to review
> > >
> > > >
> > > > }
> > > > /* return extra ref */
> > > > rpc_dplx_unref(rec,
> > > >    RPC_DPLX_FLAG_LOCKED |
> > > > RPC_DPLX_FLAG_UNLOCK);
> > > > *allocated = FALSE;
> > > >
> > > > /* return ref'd xprt */
> > > > goto done_xprt;
> > > > }
> > > > ...
> > > > xprt->xp_p1 = xd;    < but here we should increment the
> > > > refcount
> > > >
> > > > ...
> > > > }
> > > >
> > > > Both areas handle the refcount'ing wrong, but it might balance
> > > > out
> > > > sometimes.
> > > >
> > > >
> > > > What do you think ?
> > > >
> > > > Cheers Swen
> > > >
> > > >
> > >
> > > --
> > > Matt Benjamin
> > > Red Hat, Inc.
> > > 315 West Huron Street, Suite 140A
> > > Ann Arbor, Michigan 48103
> > >
> > > http://www.redhat.com/en/technologies/storage
> > >
> > > tel.  734-707-0660
> > > fax.  734-769-8938
> > > cel.  734-216-5309
> > >
> >
> 
> 
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> Nfs-ganesha-devel mailing list
> Nfs-ganesha-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

-- 
Matt Benjamin
CohortFS, LLC.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://cohortfs.com

tel.  734-761-4689 
fax.  734-769-8938 
cel.  734-216-5309 

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org!

Re: [Nfs-ganesha-devel] [ntirpc] refcount issue ?

2016-10-10 Thread Swen Schillig

On Mo, 2016-10-10 at 11:46 -0400, Matt Benjamin wrote:
> Hi Swen,
> 
> Are you going to re-push your recent closed PR as 2 new PRs as
> planned?
> 
> Dan has made some time this week to review and test, so if you have
> time, it will be easier to get to merge.
> 
> Cheers,
> 
> Matt
Hi Matt

It is re-pushed , I just didn't create a new PR yet.
I was thinking we should test and review before creating a new PR.

But if you prefer to have a PR right away, I can do that.

Cheers Swen
> 
> - Original Message -
> > 
> > From: "Matt Benjamin" 
> > To: "Swen Schillig" 
> > Cc: "Daniel Gryniewicz" , "nfs-ganesha-devel"  > -ganesha-de...@lists.sourceforge.net>
> > Sent: Tuesday, September 6, 2016 4:16:39 PM
> > Subject: Re: [ntirpc] refcount issue ?
> > 
> > Hi,
> > 
> > inline
> > 
> > - Original Message -
> > > 
> > > From: "Swen Schillig" 
> > > To: "Matt Benjamin" , "Daniel Gryniewicz"
> > > 
> > > Cc: "nfs-ganesha-devel" 
> > > Sent: Tuesday, September 6, 2016 4:10:32 PM
> > > Subject: [ntirpc] refcount issue ?
> > > 
> > > Matt, Dan.
> > > 
> > > Could you please have a look at the following code areas and
> > > verify
> > > what I think is a refcount issue.
> > > 
> > > clnt_vc_ncreate2()
> > > {
> > > ...
> > >   if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) {
> > >   xd = rec->hdl.xd = alloc_x_vc_data();
> > >   ...
> > >   } else {
> > >   xd = rec->hdl.xd;
> > >   ++(xd->refcnt);     <=== this is not right. we're not
> > > taking an addtl'
> > >   ref
> > 
> > Aren't we?  We now share a ref to previously allocated rec->hdl.xd.
> > 
> > > 
> > >   here.
> > >   }
> > > ...
> > >   clnt->cl_p1 = xd;           <=== but here we should increment
> > > the
> > >   refocunt.
> > > }
> > > 
> > > another code section with the same handling.
> > > 
> > > makefd_xprt()
> > > {
> > > ...
> > >   if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) {
> > >   newxd = true;
> > >   xd = rec->hdl.xd = alloc_x_vc_data();
> > >   ...
> > >   } else {
> > >   xd = (struct x_vc_data *)rec->hdl.xd;
> > >   /* dont return destroyed xprts */
> > >   if (!(xd->flags & X_VC_DATA_FLAG_SVC_DESTROYED)) {
> > >   if (rec->hdl.xprt) {
> > >   xprt = rec->hdl.xprt;
> > >   /* inc xprt refcnt */
> > >   SVC_REF(xprt, SVC_REF_FLAG_NONE);
> > >   } else
> > >   ++(xd->refcnt);    < not right, no
> > > addtl' ref to xd taken.
> > 
> > so this looks more likely to be incorrect, need to review
> > 
> > > 
> > >   }
> > >   /* return extra ref */
> > >   rpc_dplx_unref(rec,
> > >      RPC_DPLX_FLAG_LOCKED |
> > > RPC_DPLX_FLAG_UNLOCK);
> > >   *allocated = FALSE;
> > > 
> > >   /* return ref'd xprt */
> > >   goto done_xprt;
> > >   }
> > >   ...
> > >   xprt->xp_p1 = xd;    < but here we should increment the
> > > refcount
> > >   
> > >   ...
> > > }
> > > 
> > > Both areas handle the refcount'ing wrong, but it might balance
> > > out
> > > sometimes.
> > > 
> > > 
> > > What do you think ?
> > > 
> > > Cheers Swen
> > > 
> > > 
> > 
> > --
> > Matt Benjamin
> > Red Hat, Inc.
> > 315 West Huron Street, Suite 140A
> > Ann Arbor, Michigan 48103
> > 
> > http://www.redhat.com/en/technologies/storage
> > 
> > tel.  734-707-0660
> > fax.  734-769-8938
> > cel.  734-216-5309
> > 
> 


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

2016-10-10 Thread Malahal Naineni

I see it already merged, thank you Matt.

On Mon, Oct 10, 2016 at 9:40 PM, Malahal Naineni  wrote:
> I wanted people to review first. My tests did pass with this fix. If
> no review comments, then yes, I will do a pull request.
>
> Regards, Malahal
>
> On Mon, Oct 10, 2016 at 7:40 PM, Matt W. Benjamin  wrote:
>> Oh, it is.
>>
>> Matt
>>
>> - "Matt W. Benjamin"  wrote:
>>
>>> Hi Malahal,
>>>
>>> Is this intended to be a PR?
>>>
>>> Matt
>>>
>>> - "Malahal Naineni"  wrote:
>>>
>>> > A umount/mount loop for a krb5 mount was sending RPCSEC_GSS_DESTROY
>>> > calls at times. We end up a huge number of contexts that are
>>> expired
>>> > but
>>> > not destroyed, eventually failing mounts after some time.
>>> >
>>> > Release reference we got on GSS data object after deleting it from
>>> > hash
>>> > table.
>>> >
>>> > Signed-off-by: Malahal Naineni 
>>> > ---
>>> >  src/authgss_hash.c | 2 +-
>>> >  src/svc_auth_gss.c | 7 ++-
>>> >  2 files changed, 7 insertions(+), 2 deletions(-)
>>> >
>>> > diff --git a/src/authgss_hash.c b/src/authgss_hash.c
>>> > index d88a378..21ecaf9 100644
>>> > --- a/src/authgss_hash.c
>>> > +++ b/src/authgss_hash.c
>>> > @@ -187,7 +187,7 @@ authgss_ctx_hash_set(struct svc_rpc_gss_data
>>> *gd)
>>> > gss_ctx = (gss_union_ctx_id_desc *) (gd->ctx);
>>> > gd->hk.k = gss_ctx_hash(gss_ctx);
>>> >
>>> > -   ++(gd->refcnt); /* locked */
>>> > +   (void)atomic_inc_uint32_t(>refcnt);
>>> > t = rbtx_partition_of_scalar(_hash_st.xt, gd->hk.k);
>>> > mutex_lock(>mtx);
>>> > rslt =
>>> > diff --git a/src/svc_auth_gss.c b/src/svc_auth_gss.c
>>> > index 3322ab5..cc7b1f4 100644
>>> > --- a/src/svc_auth_gss.c
>>> > +++ b/src/svc_auth_gss.c
>>> > @@ -612,7 +612,7 @@ _svcauth_gss(struct svc_req *req, struct
>>> rpc_msg
>>> > *msg,
>>> >
>>> > *no_dispatch = true;
>>> >
>>> > -   (void)authgss_ctx_hash_del(gd); /* unrefs, can destroy gd */
>>> > +   (void)authgss_ctx_hash_del(gd);
>>> >
>>> > /* avoid lock order reversal gd->lock, xprt->xp_lock */
>>> > mutex_unlock(>lock);
>>> > @@ -622,6 +622,11 @@ _svcauth_gss(struct svc_req *req, struct
>>> rpc_msg
>>> > *msg,
>>> > svc_sendreply(req->rq_xprt, req, (xdrproc_t) xdr_void,
>>> >   (caddr_t) NULL);
>>> >
>>> > +   /* We acquired a reference on gd with authgss_ctx_hash_get
>>> > +* call.  Time to release the reference as we don't need
>>> > +* gd anymore.
>>> > +*/
>>> > +   unref_svc_rpc_gss_data(gd, SVC_RPC_GSS_FLAG_NONE);
>>> > req->rq_auth = _auth_none;
>>> >
>>> > break;
>>> > --
>>> > 1.8.3.1
>>> >
>>> >
>>> >
>>> --
>>> > Check out the vibrant tech community on one of the world's most
>>> > engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>> > ___
>>> > Nfs-ganesha-devel mailing list
>>> > Nfs-ganesha-devel@lists.sourceforge.net
>>> > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
>>>
>>> --
>>> Matt Benjamin
>>> CohortFS, LLC.
>>> 315 West Huron Street, Suite 140A
>>> Ann Arbor, Michigan 48103
>>>
>>> http://cohortfs.com
>>>
>>> tel.  734-761-4689
>>> fax.  734-769-8938
>>> cel.  734-216-5309
>>>
>>> --
>>> Check out the vibrant tech community on one of the world's most
>>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>>> ___
>>> Nfs-ganesha-devel mailing list
>>> Nfs-ganesha-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
>>
>> --
>> Matt Benjamin
>> CohortFS, LLC.
>> 315 West Huron Street, Suite 140A
>> Ann Arbor, Michigan 48103
>>
>> http://cohortfs.com
>>
>> tel.  734-761-4689
>> fax.  734-769-8938
>> cel.  734-216-5309
>>
>> --
>> Check out the vibrant tech community on one of the world's most
>> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
>> ___
>> Nfs-ganesha-devel mailing list
>> Nfs-ganesha-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] [ntirpc] refcount issue ?

2016-10-10 Thread Matt Benjamin

Hi Swen,

Are you going to re-push your recent closed PR as 2 new PRs as planned?

Dan has made some time this week to review and test, so if you have time, it 
will be easier to get to merge.

Cheers,

Matt

- Original Message -
> From: "Matt Benjamin" 
> To: "Swen Schillig" 
> Cc: "Daniel Gryniewicz" , "nfs-ganesha-devel" 
> 
> Sent: Tuesday, September 6, 2016 4:16:39 PM
> Subject: Re: [ntirpc] refcount issue ?
> 
> Hi,
> 
> inline
> 
> - Original Message -
> > From: "Swen Schillig" 
> > To: "Matt Benjamin" , "Daniel Gryniewicz"
> > 
> > Cc: "nfs-ganesha-devel" 
> > Sent: Tuesday, September 6, 2016 4:10:32 PM
> > Subject: [ntirpc] refcount issue ?
> > 
> > Matt, Dan.
> > 
> > Could you please have a look at the following code areas and verify
> > what I think is a refcount issue.
> > 
> > clnt_vc_ncreate2()
> > {
> > ...
> > if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) {
> > xd = rec->hdl.xd = alloc_x_vc_data();
> > ...
> > } else {
> > xd = rec->hdl.xd;
> > ++(xd->refcnt);     <=== this is not right. we're not taking an 
> > addtl'
> > ref
> 
> Aren't we?  We now share a ref to previously allocated rec->hdl.xd.
> 
> > here.
> > }
> > ...
> > clnt->cl_p1 = xd;           <=== but here we should increment the
> > refocunt.
> > }
> > 
> > another code section with the same handling.
> > 
> > makefd_xprt()
> > {
> > ...
> > if ((oflags & RPC_DPLX_LKP_OFLAG_ALLOC) || (!rec->hdl.xd)) {
> > newxd = true;
> > xd = rec->hdl.xd = alloc_x_vc_data();
> > ...
> > } else {
> > xd = (struct x_vc_data *)rec->hdl.xd;
> > /* dont return destroyed xprts */
> > if (!(xd->flags & X_VC_DATA_FLAG_SVC_DESTROYED)) {
> > if (rec->hdl.xprt) {
> > xprt = rec->hdl.xprt;
> > /* inc xprt refcnt */
> > SVC_REF(xprt, SVC_REF_FLAG_NONE);
> > } else
> > ++(xd->refcnt);    < not right, no addtl' 
> > ref to xd taken.
> 
> so this looks more likely to be incorrect, need to review
> 
> > }
> > /* return extra ref */
> > rpc_dplx_unref(rec,
> >    RPC_DPLX_FLAG_LOCKED | RPC_DPLX_FLAG_UNLOCK);
> > *allocated = FALSE;
> > 
> > /* return ref'd xprt */
> > goto done_xprt;
> > }
> > ...
> > xprt->xp_p1 = xd;    < but here we should increment the refcount
> > 
> > ...
> > }
> > 
> > Both areas handle the refcount'ing wrong, but it might balance out
> > sometimes.
> > 
> > 
> > What do you think ?
> > 
> > Cheers Swen
> > 
> > 
> 
> --
> Matt Benjamin
> Red Hat, Inc.
> 315 West Huron Street, Suite 140A
> Ann Arbor, Michigan 48103
> 
> http://www.redhat.com/en/technologies/storage
> 
> tel.  734-707-0660
> fax.  734-769-8938
> cel.  734-216-5309
> 

-- 
Matt Benjamin
Red Hat, Inc.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://www.redhat.com/en/technologies/storage

tel.  734-707-0660
fax.  734-769-8938
cel.  734-216-5309

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

2016-10-10 Thread Matt W. Benjamin

Hi Malahal,

Is this intended to be a PR?

Matt

- "Malahal Naineni"  wrote:

> A umount/mount loop for a krb5 mount was sending RPCSEC_GSS_DESTROY
> calls at times. We end up a huge number of contexts that are expired
> but
> not destroyed, eventually failing mounts after some time.
> 
> Release reference we got on GSS data object after deleting it from
> hash
> table.
> 
> Signed-off-by: Malahal Naineni 
> ---
>  src/authgss_hash.c | 2 +-
>  src/svc_auth_gss.c | 7 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/authgss_hash.c b/src/authgss_hash.c
> index d88a378..21ecaf9 100644
> --- a/src/authgss_hash.c
> +++ b/src/authgss_hash.c
> @@ -187,7 +187,7 @@ authgss_ctx_hash_set(struct svc_rpc_gss_data *gd)
>   gss_ctx = (gss_union_ctx_id_desc *) (gd->ctx);
>   gd->hk.k = gss_ctx_hash(gss_ctx);
>  
> - ++(gd->refcnt); /* locked */
> + (void)atomic_inc_uint32_t(>refcnt);
>   t = rbtx_partition_of_scalar(_hash_st.xt, gd->hk.k);
>   mutex_lock(>mtx);
>   rslt =
> diff --git a/src/svc_auth_gss.c b/src/svc_auth_gss.c
> index 3322ab5..cc7b1f4 100644
> --- a/src/svc_auth_gss.c
> +++ b/src/svc_auth_gss.c
> @@ -612,7 +612,7 @@ _svcauth_gss(struct svc_req *req, struct rpc_msg
> *msg,
>  
>   *no_dispatch = true;
>  
> - (void)authgss_ctx_hash_del(gd); /* unrefs, can destroy gd */
> + (void)authgss_ctx_hash_del(gd);
>  
>   /* avoid lock order reversal gd->lock, xprt->xp_lock */
>   mutex_unlock(>lock);
> @@ -622,6 +622,11 @@ _svcauth_gss(struct svc_req *req, struct rpc_msg
> *msg,
>   svc_sendreply(req->rq_xprt, req, (xdrproc_t) xdr_void,
> (caddr_t) NULL);
>  
> + /* We acquired a reference on gd with authgss_ctx_hash_get
> +  * call.  Time to release the reference as we don't need
> +  * gd anymore.
> +  */
> + unref_svc_rpc_gss_data(gd, SVC_RPC_GSS_FLAG_NONE);
>   req->rq_auth = _auth_none;
>  
>   break;
> -- 
> 1.8.3.1
> 
> 
> --
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> Nfs-ganesha-devel mailing list
> Nfs-ganesha-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

-- 
Matt Benjamin
CohortFS, LLC.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://cohortfs.com

tel.  734-761-4689 
fax.  734-769-8938 
cel.  734-216-5309 

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

2016-10-10 Thread Matt W. Benjamin

Oh, it is.

Matt

- "Matt W. Benjamin"  wrote:

> Hi Malahal,
> 
> Is this intended to be a PR?
> 
> Matt
> 
> - "Malahal Naineni"  wrote:
> 
> > A umount/mount loop for a krb5 mount was sending RPCSEC_GSS_DESTROY
> > calls at times. We end up a huge number of contexts that are
> expired
> > but
> > not destroyed, eventually failing mounts after some time.
> > 
> > Release reference we got on GSS data object after deleting it from
> > hash
> > table.
> > 
> > Signed-off-by: Malahal Naineni 
> > ---
> >  src/authgss_hash.c | 2 +-
> >  src/svc_auth_gss.c | 7 ++-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/authgss_hash.c b/src/authgss_hash.c
> > index d88a378..21ecaf9 100644
> > --- a/src/authgss_hash.c
> > +++ b/src/authgss_hash.c
> > @@ -187,7 +187,7 @@ authgss_ctx_hash_set(struct svc_rpc_gss_data
> *gd)
> > gss_ctx = (gss_union_ctx_id_desc *) (gd->ctx);
> > gd->hk.k = gss_ctx_hash(gss_ctx);
> >  
> > -   ++(gd->refcnt); /* locked */
> > +   (void)atomic_inc_uint32_t(>refcnt);
> > t = rbtx_partition_of_scalar(_hash_st.xt, gd->hk.k);
> > mutex_lock(>mtx);
> > rslt =
> > diff --git a/src/svc_auth_gss.c b/src/svc_auth_gss.c
> > index 3322ab5..cc7b1f4 100644
> > --- a/src/svc_auth_gss.c
> > +++ b/src/svc_auth_gss.c
> > @@ -612,7 +612,7 @@ _svcauth_gss(struct svc_req *req, struct
> rpc_msg
> > *msg,
> >  
> > *no_dispatch = true;
> >  
> > -   (void)authgss_ctx_hash_del(gd); /* unrefs, can destroy gd */
> > +   (void)authgss_ctx_hash_del(gd);
> >  
> > /* avoid lock order reversal gd->lock, xprt->xp_lock */
> > mutex_unlock(>lock);
> > @@ -622,6 +622,11 @@ _svcauth_gss(struct svc_req *req, struct
> rpc_msg
> > *msg,
> > svc_sendreply(req->rq_xprt, req, (xdrproc_t) xdr_void,
> >   (caddr_t) NULL);
> >  
> > +   /* We acquired a reference on gd with authgss_ctx_hash_get
> > +* call.  Time to release the reference as we don't need
> > +* gd anymore.
> > +*/
> > +   unref_svc_rpc_gss_data(gd, SVC_RPC_GSS_FLAG_NONE);
> > req->rq_auth = _auth_none;
> >  
> > break;
> > -- 
> > 1.8.3.1
> > 
> > 
> >
> --
> > Check out the vibrant tech community on one of the world's most 
> > engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> > ___
> > Nfs-ganesha-devel mailing list
> > Nfs-ganesha-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
> 
> -- 
> Matt Benjamin
> CohortFS, LLC.
> 315 West Huron Street, Suite 140A
> Ann Arbor, Michigan 48103
> 
> http://cohortfs.com
> 
> tel.  734-761-4689 
> fax.  734-769-8938 
> cel.  734-216-5309 
> 
> --
> Check out the vibrant tech community on one of the world's most 
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> Nfs-ganesha-devel mailing list
> Nfs-ganesha-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

-- 
Matt Benjamin
CohortFS, LLC.
315 West Huron Street, Suite 140A
Ann Arbor, Michigan 48103

http://cohortfs.com

tel.  734-761-4689 
fax.  734-769-8938 
cel.  734-216-5309 

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

[Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

2016-10-10 Thread Malahal Naineni

A umount/mount loop for a krb5 mount was sending RPCSEC_GSS_DESTROY
calls at times. We end up a huge number of contexts that are expired but
not destroyed, eventually failing mounts after some time.

Release reference we got on GSS data object after deleting it from hash
table.

Signed-off-by: Malahal Naineni 
---
 src/authgss_hash.c | 2 +-
 src/svc_auth_gss.c | 7 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/authgss_hash.c b/src/authgss_hash.c
index d88a378..21ecaf9 100644
--- a/src/authgss_hash.c
+++ b/src/authgss_hash.c
@@ -187,7 +187,7 @@ authgss_ctx_hash_set(struct svc_rpc_gss_data *gd)
gss_ctx = (gss_union_ctx_id_desc *) (gd->ctx);
gd->hk.k = gss_ctx_hash(gss_ctx);
 
-   ++(gd->refcnt); /* locked */
+   (void)atomic_inc_uint32_t(>refcnt);
t = rbtx_partition_of_scalar(_hash_st.xt, gd->hk.k);
mutex_lock(>mtx);
rslt =
diff --git a/src/svc_auth_gss.c b/src/svc_auth_gss.c
index 3322ab5..cc7b1f4 100644
--- a/src/svc_auth_gss.c
+++ b/src/svc_auth_gss.c
@@ -612,7 +612,7 @@ _svcauth_gss(struct svc_req *req, struct rpc_msg *msg,
 
*no_dispatch = true;
 
-   (void)authgss_ctx_hash_del(gd); /* unrefs, can destroy gd */
+   (void)authgss_ctx_hash_del(gd);
 
/* avoid lock order reversal gd->lock, xprt->xp_lock */
mutex_unlock(>lock);
@@ -622,6 +622,11 @@ _svcauth_gss(struct svc_req *req, struct rpc_msg *msg,
svc_sendreply(req->rq_xprt, req, (xdrproc_t) xdr_void,
  (caddr_t) NULL);
 
+   /* We acquired a reference on gd with authgss_ctx_hash_get
+* call.  Time to release the reference as we don't need
+* gd anymore.
+*/
+   unref_svc_rpc_gss_data(gd, SVC_RPC_GSS_FLAG_NONE);
req->rq_auth = _auth_none;
 
break;
-- 
1.8.3.1


--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Nfs-ganesha-devel mailing list
Nfs-ganesha-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel

Re: [Nfs-ganesha-devel] [ntirpc] refcount issue ?

Re: [Nfs-ganesha-devel] [ntirpc] refcount issue ?

Re: [Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

Re: [Nfs-ganesha-devel] [ntirpc] refcount issue ?

Re: [Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

Re: [Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

[Nfs-ganesha-devel] [PATCH] Fix refcount leak in svc_rpc_gss_data under RPCSEC_GSS_DESTROY

7 matches

Site Navigation

Mail list logo

Footer information