Bug 242712

Summary: Networking device detach leaks memory after base r334118
Product: Base System Reporter: ghuckriede
Component: kernAssignee: Mark Johnston <markj>
Status: Closed FIXED    
Severity: Affects Many People CC: afedorov, bz, emaste, hselasky, markj, mmacy, net
Priority: --- Keywords: regression
Version: CURRENTFlags: koobs: mfc-stable12+
koobs: mfc-stable11-
Hardware: Any   
OS: Any   
Attachments:
Description Flags
before and after test results none

Description ghuckriede 2019-12-18 18:50:16 UTC 
Created attachment 210040 [details]
before and after test results

A free() appears to have been removed by mistake.

line 1544 of https://svnweb.freebsd.org/base/head/sys/net/if.c?annotate=334117
was removed by the following submission:	
https://svnweb.freebsd.org/base/head/sys/net/if.c?revision=334118&view=markup

This introduced a memory leak when calling if_delgroups(), as the malloc is still present in if_addgroup() on line 1458 of https://svnweb.freebsd.org/base/head/sys/net/if.c?annotate=355070

Furthermore the free() is present for if_delgroup() on line 1551 of https://svnweb.freebsd.org/base/head/sys/net/if.c?annotate=355070

Here is a fix along with a white space change.

svn diff sys/net/if.c
Index: sys/net/if.c
===================================================================
--- sys/net/if.c	(revision 355854)
+++ sys/net/if.c	(working copy)
@@ -1594,9 +1594,10 @@
 		free(ifgm, M_TEMP);
 		if (ifglfree) {
 			EVENTHANDLER_INVOKE(group_detach_event,
-								ifgl->ifgl_group);
+			                    ifgl->ifgl_group);
 			free(ifgl->ifgl_group, M_TEMP);
 		}
+		free(ifgl, M_TEMP);
 		EVENTHANDLER_INVOKE(group_change_event, groupname);
 
 		IFNET_WLOCK();
Comment 1 Mark Johnston freebsd_committer freebsd_triage 2019-12-20 16:55:35 UTC 
Nice find, thank you.  This code is largely duplicated between if_delgroup() and if_delgroups() and should be merged.
Comment 2 commit-hook freebsd_committer freebsd_triage 2019-12-20 17:22:12 UTC 
A commit references this bug:

Author: markj
Date: Fri Dec 20 17:21:58 UTC 2019
New revision: 355938
URL: https://svnweb.freebsd.org/changeset/base/355938

Log:
  Fix a memory leak in if_delgroups() introduced in r334118.

  PR:		242712
  Submitted by:	ghuckriede@blackberry.com
  MFC after:	3 days

Changes:
  head/sys/net/if.c
Comment 3 Mark Johnston freebsd_committer freebsd_triage 2019-12-20 17:22:51 UTC 
I posted a larger cleanup here if anyone on the CC wants to take a look: https://reviews.freebsd.org/D22892
Comment 4 commit-hook freebsd_committer freebsd_triage 2019-12-23 16:35:40 UTC 
A commit references this bug:

Author: markj
Date: Mon Dec 23 16:34:40 UTC 2019
New revision: 356037
URL: https://svnweb.freebsd.org/changeset/base/356037

Log:
  MFC r355938:
  Fix a memory leak in if_delgroups() introduced in r334118.

  PR:	242712

Changes:
_U  stable/12/
  stable/12/sys/net/if.c