Correct.
Matt
On Thu, Aug 3, 2017 at 10:44 AM, Daniel Gryniewicz <d...@redhat.com> wrote:
> I do not believe it's reorganizing code. I think the code in the first
> section is wrong, and the second version is needed.
>
> Once you've dec'd the refcount, you cannot access the pointer. At that
> point, another thread could deref and free, and you'd be left with freed
> memory. You *must* store that value in a local variable.
>
> Daniel
>
>
> On 08/03/2017 02:13 AM, William Allen Simpson wrote:
>>
>> It's improperly reorganizing code.
>>
>> ===
>>
>> Accessing freed memory at ==>
>>
>> Logically, this couldn't happen!
>>
>> int free_nfs_request(request_data_t *reqdata)
>> {
>> atomic_dec_uint32_t(&reqdata->r_d_refs);
>>
>> LogDebug(COMPONENT_DISPATCH,
>> "%s: %p fd %d xp_refs %" PRIu32 " r_d_refs %" PRIu32,
>> __func__,
>> reqdata->r_u.req.svc.rq_xprt,
>> reqdata->r_u.req.svc.rq_xprt->xp_fd,
>> reqdata->r_u.req.svc.rq_xprt->xp_refs,
>> reqdata->r_d_refs);
>>
>> if (reqdata->r_d_refs)
>> ==> return reqdata->r_d_refs;
>>
>> switch (reqdata->rtype) {
>> case NFS_REQUEST:
>> /* dispose RPC header */
>> if (reqdata->r_u.req.svc.rq_auth)
>> SVCAUTH_RELEASE(&(reqdata->r_u.req.svc));
>> XDR_DESTROY(reqdata->r_u.req.svc.rq_xdrs);
>> break;
>> default:
>> break;
>> }
>> SVC_RELEASE(reqdata->r_u.req.svc.rq_xprt, SVC_RELEASE_FLAG_NONE);
>> pool_free(request_pool, reqdata);
>> return 0;
>> }
>>
>> ===
>>
>> This works by using local variables. Kinda pointlessly, when not
>> running log debug.
>>
>> int free_nfs_request(request_data_t *reqdata)
>> {
>> SVCXPRT *xprt = reqdata->r_u.req.svc.rq_xprt;
>> uint32_t refs = atomic_dec_uint32_t(&reqdata->r_d_refs);
>>
>> LogDebug(COMPONENT_DISPATCH,
>> "%s: %p fd %d xp_refs %" PRIu32 " r_d_refs %" PRIu32,
>> __func__,
>> xprt, xprt->xp_fd, xprt->xp_refs,
>> refs);
>>
>> if (refs)
>> return refs;
>>
>> switch (reqdata->rtype) {
>> case NFS_REQUEST:
>> /* dispose RPC header */
>> if (reqdata->r_u.req.svc.rq_auth)
>> SVCAUTH_RELEASE(&(reqdata->r_u.req.svc));
>> XDR_DESTROY(reqdata->r_u.req.svc.rq_xdrs);
>> break;
>> default:
>> break;
>> }
>> SVC_RELEASE(xprt, SVC_RELEASE_FLAG_NONE);
>> pool_free(request_pool, reqdata);
>> return 0;
>> }
>>
>>
>> ------------------------------------------------------------------------------
>> 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
------------------------------------------------------------------------------
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
