diff mbox

batadv-vis: don't leak socket fd in get_if_mac()

Message ID d48019225b19f19f5ac724d3b1a247cdf921f92b.1390143639.git.mschiffer@universe-factory.net (mailing list archive)
State Accepted, archived
Commit e99382edc23ece11c0ad1fbd91aaefeb222c9cb4
Headers

Commit Message

Matthias Schiffer Jan. 19, 2014, 3:01 p.m. UTC 
  Leaking an fd every time get_if_mac() is called causes a batadv-vis server
process to hit the open file limit in a matter of hours when it is as low as
1024 (which it is on OpenWRT).

Reported-by: Jan-Philipp Litza <janphilipp@litza.de>
Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
---
 vis/vis.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Antonio Quartulli Jan. 19, 2014, 4:16 p.m. UTC | #1 
On 19/01/14 16:01, Matthias Schiffer wrote:
> Leaking an fd every time get_if_mac() is called causes a batadv-vis server
> process to hit the open file limit in a matter of hours when it is as low as
> 1024 (which it is on OpenWRT).
> 
> Reported-by: Jan-Philipp Litza <janphilipp@litza.de>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>

Matthias,

what repository is this patch against?

> ---
>  vis/vis.c | 8 ++++++--

I am puzzled because your commit subject says "batadv-vis" but the patch
does not seem to be against batman-adv/batctl. Can you clarify please?

Cheers,
Matthias Schiffer Jan. 19, 2014, 4:43 p.m. UTC | #2 
On 01/19/2014 05:16 PM, Antonio Quartulli wrote:
> On 19/01/14 16:01, Matthias Schiffer wrote:
>> Leaking an fd every time get_if_mac() is called causes a batadv-vis server
>> process to hit the open file limit in a matter of hours when it is as low as
>> 1024 (which it is on OpenWRT).
>>
>> Reported-by: Jan-Philipp Litza <janphilipp@litza.de>
>> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>
> 
> Matthias,
> 
> what repository is this patch against?
> 
>> ---
>>  vis/vis.c | 8 ++++++--
> 
> I am puzzled because your commit subject says "batadv-vis" but the patch
> does not seem to be against batman-adv/batctl. Can you clarify please?
> 
> Cheers,
> 
> 
> 

Sorry, I forgot to change the subject to [PATCH alfred]...
Simon Wunderlich Jan. 20, 2014, 10:52 a.m. UTC | #3 
> Leaking an fd every time get_if_mac() is called causes a batadv-vis server
> process to hit the open file limit in a matter of hours when it is as low
> as 1024 (which it is on OpenWRT).
> 
> Reported-by: Jan-Philipp Litza <janphilipp@litza.de>
> Signed-off-by: Matthias Schiffer <mschiffer@universe-factory.net>

Applied in e99382e. If you use something like "alfred" in the header next time 
it will make it more unlikely for me to miss your patch. :)

Thanks!
    Simon
diff mbox

Patch

diff --git a/vis/vis.c b/vis/vis.c
index dcb4db4..7a8e780 100644
--- a/vis/vis.c
+++ b/vis/vis.c
@@ -96,7 +96,7 @@  static uint8_t *str_to_mac(char *str)
 static int get_if_mac(char *ifname, uint8_t *mac)
 {
 	struct ifreq ifr;
-	int sock;
+	int sock, ret;
 
 	strncpy(ifr.ifr_name, ifname, IFNAMSIZ);
 
@@ -105,7 +105,11 @@  static int get_if_mac(char *ifname, uint8_t *mac)
 		return -1;
 	}
 
-	if (ioctl(sock, SIOCGIFHWADDR, &ifr) == -1) {
+	ret = ioctl(sock, SIOCGIFHWADDR, &ifr);
+
+	close(sock);
+
+	if (ret == -1) {
 		fprintf(stderr, "can't get MAC address: %s\n", strerror(errno));
 		return -1;
 	}