Bug 242712 - Networking device detach leaks memory after base r334118
Summary: Networking device detach leaks memory after base r334118
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Mark Johnston
URL:
Keywords: regression
Depends on:
Blocks:
 
Reported: 2019-12-18 18:50 UTC by ghuckriede
Modified: 2019-12-23 23:57 UTC (History)
7 users (show)

See Also:
koobs: mfc-stable12+
koobs: mfc-stable11-


Attachments
before and after test results (3.83 KB, text/plain)
2019-12-18 18:50 UTC, ghuckriede
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
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 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 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 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 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