[prev in list] [next in list] [prev in thread] [next in thread] 

List:       trousers-tech
Subject:    Re: [TrouSerS-tech] [PATCH 12/17] Fixed memory leak in function Transport_TerminateHandle
From:       Richard <rmaciel () linux ! vnet ! ibm ! com>
Date:       2014-04-14 21:39:55
Message-ID: 534C55AB.2020605 () linux ! vnet ! ibm ! com
[Download RAW message or body]

I take back my assertion! :-)

There is no problem with the double-free, because the 
RPC_ExecuteTransport_TP sets the pointer to NULL.

However, I found something significant by reanalyzing: 
Transport_TerminateHandle allocates memory that, ultimately, goes to the 
RPC_ExecuteTransport_TP. The latter uses that memory, but never frees 
it. Even worse: it allocates more memory and stores in the same pointer, 
effectively losing track of the memory allocated by the former function.

If you're having a hard time figuring out what I'm describing here, 
check the src/tspi/rpc/tcstp/rpc_transport.c:191.

So, my idea here is to properly free the memory allocated by 
RPC_ExecuteTransport_TP in the aforementioned position and set the 
pointer used to keep track of the allocation to NULL. That way, in case 
of an error, the free function (in Transport_TerminateHandle) will 
ignore it.

Em 11-04-2014 11:17, Richard escreveu:
> Em 11-04-2014 06:51, Fuchs, Andreas escreveu:
> > Disclaimer:
> > I could not complie-test or runtime-test these patches right now. This is a pure \
> > code-only review of the patches. 
> > The patch is not correct. The free may only be performed in case of result == \
> > TSS2_SUCCESS. The current version has a potential double-free. In case of error, \
> > RPC_ExecuteTransport_TP will falsely free this memory. 
> > This needs to be fixed inside RPC_ExecuteTransport_TP then this patch would be \
> > correct, but a bunch of other places would probably need to be adjusted as well. 
> > This is going to be a major rewrite I fear...
> You're absolutely right. In fact, the dynamics of the
> Transport_TerminateHandle are very weird. One of the functions called by
> obj_context_transport_execute already does memory allocation (and
> doesn't free the memory coming from the former).
> 
> I'll take some more time to think about this one.
> 
> > Am Mittwoch, den 09.04.2014, 15:41 -0300 schrieb rmaciel@linux.vnet.ibm.com:
> > > From: Richard Maciel <rmaciel@linux.vnet.ibm.com>
> > > 
> > > Related to coverity CID 10307.
> > > 
> > > Pointer handles got memory allocated for it, but that memory is never
> > > freed at the end of the function.
> > > 
> > > Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com>
> > > ---
> > > src/tspi/tsp_auth.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/src/tspi/tsp_auth.c b/src/tspi/tsp_auth.c
> > > index 5ee6f5d..f11fa60 100755
> > > --- a/src/tspi/tsp_auth.c
> > > +++ b/src/tspi/tsp_auth.c
> > > @@ -1225,6 +1225,8 @@ Transport_TerminateHandle(TSS_HCONTEXT tspContext, /* in \
> > > */  result = obj_context_transport_execute(tspContext, \
> > > TPM_ORD_Terminate_Handle, 0, NULL,  NULL, &handlesLen, &handles, NULL, NULL, \
> > > NULL, NULL); 
> > > +	free(handles);
> > > +
> > > 	return result;
> > > }
> > > #endif
> 
> ------------------------------------------------------------------------------
> Put Bad Developers to Shame
> Dominate Development with Jenkins Continuous Integration
> Continuously Automate Build, Test & Deployment
> Start a new project now. Try Jenkins in the cloud.
> http://p.sf.net/sfu/13600_Cloudbees
> _______________________________________________
> TrouSerS-tech mailing list
> TrouSerS-tech@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/trousers-tech
> 


------------------------------------------------------------------------------
Learn Graph Databases - Download FREE O'Reilly Book
"Graph Databases" is the definitive new guide to graph databases and their
applications. Written by three acclaimed leaders in the field,
this first edition is now available. Download your free book today!
http://p.sf.net/sfu/NeoTech
_______________________________________________
TrouSerS-tech mailing list
TrouSerS-tech@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/trousers-tech


[prev in list] [next in list] [prev in thread] [next in thread]