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

List:       trousers-tech
Subject:    Re: [TrouSerS-tech] [PATCH 04/17] Fixed modules where unitialized pointers could be freed
From:       "Fuchs, Andreas" <andreas.fuchs () sit ! fraunhofer ! de>
Date:       2014-04-11 13:19:01
Message-ID: 1397222341.15139.50.camel () pc-fuchslap2 ! sit ! fraunhofer ! de
[Download RAW message or body]

Of course, you are right, I completely messed up here. That
NULL-initialization is of course necessary for the first error-handler.

What I don't understand though is the "res = NULL" in line 486. That
still seems unnecessary.


Am Freitag, den 11.04.2014, 09:34 -0300 schrieb Richard:
> Em 11-04-2014 06:43, 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. 
> > src/tcs/tcsi_ps.c:
> > This seems to be correct, though I'd instead move the free into the braces, where \
> > the malloc is guaranteed to have initialized the pubkey.key-field. 
> > src/tspi/rpc/tcstp/rpc.c:
> > This is actually unnecessary, since for rv != 0, res is set to NULL in the \
> > rv-handler. If it makes the code-scanner happy, the patch is ok, but I'd remove \
> > line 486 "res = NULL;" from the rv-handler
> 
> Actually this is there for the error handling of "if (get_tcsd_port... 
> )" - line 475. When it goes to exit, there is a test in order to verify 
> if the addrinfo linked list must be free. The "res = NULL" is there for 
> a similar reason.
> 
> It's basically just to be safe, because I don't know if freeaddrinfo is 
> able to deal with a NULL parameter. Its manual page doesn't state it 
> (opposed to the free man page which explicitly lets you know that it's a 
> valid behavior).
> > 
> > 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 coverit CIDs 10326 and 10323
> > > 
> > > In both cases proper pointer initialization was not
> > > made, so, in some cases, the code could free the value of a
> > > unitialized pointer.
> > > 
> > > Signed-off-by: Richard Maciel <rmaciel@linux.vnet.ibm.com>
> > > ---
> > > src/tcs/tcsi_ps.c        | 2 ++
> > > src/tspi/rpc/tcstp/rpc.c | 2 +-
> > > 2 files changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/tcs/tcsi_ps.c b/src/tcs/tcsi_ps.c
> > > index 87db219..e7f6245 100644
> > > --- a/src/tcs/tcsi_ps.c
> > > +++ b/src/tcs/tcsi_ps.c
> > > @@ -610,6 +610,8 @@ \
> > > TCSP_GetRegisteredKeyByPublicInfo_Internal(TCS_CONTEXT_HANDLE tcsContext,	/* in \
> > > TCPA_STORE_PUBKEY pubKey;  TSS_RESULT result = TCSERR(TSS_E_FAIL);
> > > 
> > > +	pubKey.key = NULL;
> > > +
> > > 	if ((result = ctx_verify_context(tcsContext)))
> > > 		return result;
> > > 
> > > diff --git a/src/tspi/rpc/tcstp/rpc.c b/src/tspi/rpc/tcstp/rpc.c
> > > index afe1844..b54ca2f 100644
> > > --- a/src/tspi/rpc/tcstp/rpc.c
> > > +++ b/src/tspi/rpc/tcstp/rpc.c
> > > @@ -462,7 +462,7 @@ TSS_RESULT
> > > get_socket(struct host_table_entry *hte, int *sd)
> > > {
> > > 	char port_str[TCP_PORT_STR_MAX_LEN]; // To accomodate string 65535
> > > -	struct addrinfo hints, *res, *p;
> > > +	struct addrinfo hints, *p, *res=NULL;
> > > 	int rv;
> > > 	TSS_RESULT result = TSS_SUCCESS;
> > > 
> 

------------------------------------------------------------------------------
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


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