19a0061b6Sopenharmony_ciFrom fb63f8b7337aa11a667537e6a3b399062ede2eb5 Mon Sep 17 00:00:00 2001
29a0061b6Sopenharmony_ciFrom: Phil Sutter <phil@nwl.cc>
39a0061b6Sopenharmony_ciDate: Fri, 25 Nov 2022 21:35:28 +0100
49a0061b6Sopenharmony_ciSubject: [PATCH] iptables: Plug memleaks in print_firewall()
59a0061b6Sopenharmony_ci
69a0061b6Sopenharmony_ciWhen adding a rule in verbose mode, valgrind prints:
79a0061b6Sopenharmony_ci
89a0061b6Sopenharmony_ci192 bytes in 1 blocks are definitely lost in loss record 1 of 2
99a0061b6Sopenharmony_ci   at 0x48417E5: malloc (vg_replace_malloc.c:381)
109a0061b6Sopenharmony_ci   by 0x486B158: xtables_malloc (xtables.c:446)
119a0061b6Sopenharmony_ci   by 0x486C1F6: xtables_find_match (xtables.c:826)
129a0061b6Sopenharmony_ci   by 0x10E684: print_match (iptables.c:115)
139a0061b6Sopenharmony_ci   by 0x10E684: print_firewall (iptables.c:169)
149a0061b6Sopenharmony_ci   by 0x10FC0C: print_firewall_line (iptables.c:196)
159a0061b6Sopenharmony_ci   by 0x10FC0C: append_entry (iptables.c:221)
169a0061b6Sopenharmony_ci   by 0x10FC0C: do_command4 (iptables.c:776)
179a0061b6Sopenharmony_ci   by 0x10E45B: iptables_main (iptables-standalone.c:59)
189a0061b6Sopenharmony_ci   by 0x49A2349: (below main) (in /lib64/libc.so.6)
199a0061b6Sopenharmony_ci
209a0061b6Sopenharmony_ci200 bytes in 1 blocks are definitely lost in loss record 2 of 2
219a0061b6Sopenharmony_ci   at 0x48417E5: malloc (vg_replace_malloc.c:381)
229a0061b6Sopenharmony_ci   by 0x486B158: xtables_malloc (xtables.c:446)
239a0061b6Sopenharmony_ci   by 0x486BBD6: xtables_find_target (xtables.c:956)
249a0061b6Sopenharmony_ci   by 0x10E579: print_firewall (iptables.c:145)
259a0061b6Sopenharmony_ci   by 0x10FC0C: print_firewall_line (iptables.c:196)
269a0061b6Sopenharmony_ci   by 0x10FC0C: append_entry (iptables.c:221)
279a0061b6Sopenharmony_ci   by 0x10FC0C: do_command4 (iptables.c:776)
289a0061b6Sopenharmony_ci   by 0x10E45B: iptables_main (iptables-standalone.c:59)
299a0061b6Sopenharmony_ci   by 0x49A2349: (below main) (in /lib64/libc.so.6)
309a0061b6Sopenharmony_ci
319a0061b6Sopenharmony_ciIf the match/target was cloned, it needs to be freed. Basically a bug since
329a0061b6Sopenharmony_ciday 1.
339a0061b6Sopenharmony_ci
349a0061b6Sopenharmony_ciConflict: NA
359a0061b6Sopenharmony_ciReference: https://git.netfilter.org/iptables/commit?id=fb63f8b7337aa11a667537e6a3b399062ede2eb5
369a0061b6Sopenharmony_ci
379a0061b6Sopenharmony_ciSigned-off-by: Phil Sutter <phil@nwl.cc>
389a0061b6Sopenharmony_ci---
399a0061b6Sopenharmony_ci iptables/ip6tables.c | 6 ++++++
409a0061b6Sopenharmony_ci iptables/iptables.c  | 6 ++++++
419a0061b6Sopenharmony_ci 2 files changed, 12 insertions(+)
429a0061b6Sopenharmony_ci
439a0061b6Sopenharmony_cidiff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
449a0061b6Sopenharmony_ciindex 062b2b15..1d232657 100644
459a0061b6Sopenharmony_ci--- a/iptables/ip6tables.c
469a0061b6Sopenharmony_ci+++ b/iptables/ip6tables.c
479a0061b6Sopenharmony_ci@@ -122,6 +122,9 @@ print_match(const struct xt_entry_match *m,
489a0061b6Sopenharmony_ci 			printf("%s%s ", match->name, unsupported_rev);
499a0061b6Sopenharmony_ci 		else
509a0061b6Sopenharmony_ci 			printf("%s ", match->name);
519a0061b6Sopenharmony_ci+
529a0061b6Sopenharmony_ci+		if (match->next == match)
539a0061b6Sopenharmony_ci+			free(match);
549a0061b6Sopenharmony_ci 	} else {
559a0061b6Sopenharmony_ci 		if (name[0])
569a0061b6Sopenharmony_ci 			printf("UNKNOWN match `%s' ", name);
579a0061b6Sopenharmony_ci@@ -179,6 +182,9 @@ print_firewall(const struct ip6t_entry *fw,
589a0061b6Sopenharmony_ci 			tg->print(&fw->ipv6, t, format & FMT_NUMERIC);
599a0061b6Sopenharmony_ci 		else if (target->print)
609a0061b6Sopenharmony_ci 			printf(" %s%s", target->name, unsupported_rev);
619a0061b6Sopenharmony_ci+
629a0061b6Sopenharmony_ci+		if (target->next == target)
639a0061b6Sopenharmony_ci+			free(target);
649a0061b6Sopenharmony_ci 	} else if (t->u.target_size != sizeof(*t))
659a0061b6Sopenharmony_ci 		printf("[%u bytes of unknown target data] ",
669a0061b6Sopenharmony_ci 		       (unsigned int)(t->u.target_size - sizeof(*t)));
679a0061b6Sopenharmony_cidiff --git a/iptables/iptables.c b/iptables/iptables.c
689a0061b6Sopenharmony_ciindex 0351b39f..d246198f 100644
699a0061b6Sopenharmony_ci--- a/iptables/iptables.c
709a0061b6Sopenharmony_ci+++ b/iptables/iptables.c
719a0061b6Sopenharmony_ci@@ -122,6 +122,9 @@ print_match(const struct xt_entry_match *m,
729a0061b6Sopenharmony_ci 			printf("%s%s ", match->name, unsupported_rev);
739a0061b6Sopenharmony_ci 		else
749a0061b6Sopenharmony_ci 			printf("%s ", match->name);
759a0061b6Sopenharmony_ci+
769a0061b6Sopenharmony_ci+		if (match->next == match)
779a0061b6Sopenharmony_ci+			free(match);
789a0061b6Sopenharmony_ci 	} else {
799a0061b6Sopenharmony_ci 		if (name[0])
809a0061b6Sopenharmony_ci 			printf("UNKNOWN match `%s' ", name);
819a0061b6Sopenharmony_ci@@ -178,6 +181,9 @@ print_firewall(const struct ipt_entry *fw,
829a0061b6Sopenharmony_ci 			tg->print(&fw->ip, t, format & FMT_NUMERIC);
839a0061b6Sopenharmony_ci 		else if (target->print)
849a0061b6Sopenharmony_ci 			printf(" %s%s", target->name, unsupported_rev);
859a0061b6Sopenharmony_ci+
869a0061b6Sopenharmony_ci+		if (target->next == target)
879a0061b6Sopenharmony_ci+			free(target);
889a0061b6Sopenharmony_ci 	} else if (t->u.target_size != sizeof(*t))
899a0061b6Sopenharmony_ci 		printf("[%u bytes of unknown target data] ",
909a0061b6Sopenharmony_ci 		       (unsigned int)(t->u.target_size - sizeof(*t)));
919a0061b6Sopenharmony_ci-- 
929a0061b6Sopenharmony_ci2.23.0
93