[pgpool-hackers: 3704] Re: SSL memory leak

[Bo Peng]

Ishii-san,

I have fixed this memory leak.

https://git.postgresql.org/gitweb/?p=pgpool2.git;a=commit;h=0fdbbb3698ab91640917e19c887e78d572c725cf

On Mon, 29 Jun 2020 09:05:47 +0900
Bo Peng <pengbo at sraoss.co.jp> wrote:

> Hi Ishii-san,
> 
> On Sat, 27 Jun 2020 21:29:34 +0900 (JST)
> Tatsuo Ishii <ishii at sraoss.co.jp> wrote:
> 
> > I have run Coverity against master branch head. There are some memory
> > leak errors in src/utils/pool_ssl.c.
> > 
> > > *** CID 1429988:    (RESOURCE_LEAK)
> > > /src/utils/pool_ssl.c: 360 in init_ssl_ctx()
> > > 354     
> > > 355     		if (cacert || cacert_dir)
> > > 356     		{
> > > 357     			error = SSL_CTX_load_verify_locations(cp->ssl_ctx,
> > > 358     												  cacert,
> > > 359     												  cacert_dir);
> > >>>>     CID 1429988:    (RESOURCE_LEAK)
> > >>>>     Variable "conf_file_copy" going out of scope leaks the storage it points to.
> > > 360     			SSL_RETURN_ERROR_IF((error != 1), "SSL verification setup");
> > > 361     			SSL_CTX_set_verify(cp->ssl_ctx, SSL_VERIFY_PEER, NULL);
> > > 362     		}
> > > 363     	}
> > 
> > This was introduced by this commit:
> > --------------------------------------------------
> > 	Mon, 18 May 2020 21:12:25 +0900 (21:12 +0900)
> > committer	Bo Peng <pengbo at sraoss.co.jp>	
> > commit	fc9e9d3733a9c2c14c10bb3af25217f386ee41c7
> > 
> > Change relative path of SSL files to configuration directory.
> > 
> > Patch is created by Umar Hayat and Japanese documentation is added by me.
> > --------------------------------------------------
> > 
> > Macro SSL_RETURN_ERROR_IF is actually:
> > 
> > #define SSL_RETURN_ERROR_IF(cond, msg) \
> > 	do { \
> > 		if ( (cond) ) { \
> > 			perror_ssl( (msg) ); \
> > 			return -1; \
> > 		} \
> > 	} while (0);
> > 
> > The leaking storage is this:
> > 
> > 	char *conf_file_copy = pstrdup(get_config_file_name());
> > 
> > When SSL_RETURN_ERROR_IF is called, conf_file_copy is not freed, which
> > is what Coverity is complaining. Quick and dirty fix would be
> > something like:
> > 
> > #define SSL_RETURN_ERROR_IF(cond, msg) \
> > 	do { \
> > 		if ( (cond) ) { \
> > 			perror_ssl( (msg) ); \
> > 			pfree(conf_file_copy); \
> > 			return -1; \
> > 		} \
> > 	} while (0);
> > 
> > Peng, What do you think?
> 
> I will look into this one.
>  
> > Best regards,
> > --
> > Tatsuo Ishii
> > SRA OSS, Inc. Japan
> > English: http://www.sraoss.co.jp/index_en.php
> > Japanese:http://www.sraoss.co.jp
> > _______________________________________________
> > pgpool-hackers mailing list
> > pgpool-hackers at pgpool.net
> > http://www.pgpool.net/mailman/listinfo/pgpool-hackers
> 
> 
> -- 
> Bo Peng <pengbo at sraoss.co.jp>
> SRA OSS, Inc. Japan
> _______________________________________________
> pgpool-hackers mailing list
> pgpool-hackers at pgpool.net
> http://www.pgpool.net/mailman/listinfo/pgpool-hackers


-- 
Bo Peng <pengbo at sraoss.co.jp>
SRA OSS, Inc. Japan