Date: Fri, 18 May 2018 16:46:18 +1000 From: Lawrence Stewart <lstewart@freebsd.org> To: Tom Jones <tj@enoti.me>, Harsh Jain <harsh@chelsio.com> Cc: freebsd-net@freebsd.org, Navdeep Parhar <navdeep@chelsio.com>, sonyarpitad@chelsio.com, John Baldwin <jhb@freebsd.org> Subject: Re: Bug: Newreno; Seems Memory leak in newreno_cb_init Message-ID: <6e4d875e-15db-51f9-2031-82b2bb631f08@freebsd.org> In-Reply-To: <6c84c94a-9d16-db30-f8d5-c6b364cd9469@freebsd.org> References: <58ab6c13-f9c4-b4d7-7f2c-eade3749457f@chelsio.com> <20180508200439.GA32339@tom-desk.erg.abdn.ac.uk> <6c84c94a-9d16-db30-f8d5-c6b364cd9469@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 09/05/2018 11:00, Lawrence Stewart wrote: > On 09/05/2018 06:04, Tom Jones wrote: >> On Tue, May 08, 2018 at 05:14:49PM +0530, Harsh Jain wrote: >>> Hi All, >>> >>> We have observed memory leak with TCP network traffic in "newreno". >>> >>> Output of vmstat -m >>> >>> in_mfilter 3 3K - 3 1024 >>> in_multi 4 1K - 4 256 >>> ip_moptions 6 1K - 6 64,256 >>> encap_export_host 2 2K - 2 1024 >>> newreno data 394849273 6169520K - 394849273 16 >>> sctp_a_it 0 0K - 5 16 >>> sctp_vrf 1 1K - 1 64 >>> sctp_ifa 7 1K - 7 128 >>> sctp_ifn 4 1K - 4 128 >>> >>> There is 1 malloc in "newreno_cb_init" whose pointer is not saved in any global structure to free the same. >>> >>> Is this a BUG? >> >> >> Hi Harsh, >> >> Adding Lawrence in cc >> >> It looks like it, running nc in a loop I can watch MemUse grow. >> >> I think this should address the leak >> >> https://reviews.freebsd.org/D15358 > > I'm not clear why yet, but the patch I ultimately committed as r331214 > is deeply flawed on account of missing memory allocation and other > changes that never ended up in the working copy I committed from. The > cb_destroy() change for example exists in the D11616 Phabricator review > though. I think I may have refined the final patch and committed from a > working copy that started with an older stale version of the patch. Ugh. > > Mea culpa, thanks for the bug report, and apologies for the oversight. > Will work with Tom to get this fixed. Forgot to mention, this should be fixed with the commit of r333699: > Author: lstewart > Date: Thu May 17 02:46:27 2018 > New Revision: 333699 > URL: https://svnweb.freebsd.org/changeset/base/333699 > > Log: > Plug a memory leak and potential NULL-pointer dereference introduced in r331214. > > Each TCP connection that uses the system default cc_newreno(4) congestion > control algorithm module leaks a "struct newreno" (8 bytes of memory) at > connection initialisation time. The NULL-pointer dereference is only germane > when using the ABE feature, which is disabled by default. > > While at it: > > - Defer the allocation of memory until it is actually needed given that ABE is > optional and disabled by default. > > - Document the ENOMEM errno in getsockopt(2)/setsockopt(2). > > - Document ENOMEM and ENOBUFS in tcp(4) as being synonymous given that they are > used interchangeably throughout the code. > > - Fix a few other nits also accidentally omitted from the original patch. > > Reported by: Harsh Jain on freebsd-net@ > Tested by: tjh@ > Differential Revision: https://reviews.freebsd.org/D15358 > > Modified: > head/lib/libc/sys/getsockopt.2 > head/share/man/man4/tcp.4 > head/sys/netinet/cc/cc_newreno.c
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6e4d875e-15db-51f9-2031-82b2bb631f08>