[iproute PATCH v2 0/2] Covscan: Fix potential file descriptor leaks

[iproute PATCH v2 0/2] Covscan: Fix potential file descriptor leaks

[Phil Sutter]

This series collects patches from v1 which deal with potential file
descriptor leaks.

No changes to the actual patches, just splitting into smaller series.

Phil Sutter (2):
  ss: Don't leak fd in tcp_show_netlink_file()
  tc/em_ipset: Don't leak sockfd on error path

 misc/ss.c     | 32 ++++++++++++++++++++------------
 tc/em_ipset.c |  1 +
 2 files changed, 21 insertions(+), 12 deletions(-)

-- 
2.13.1

[iproute PATCH v2 1/2] ss: Don't leak fd in tcp_show_netlink_file()

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index d767b1103ea81..07eecfa7a36db 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2687,41 +2687,44 @@ static int tcp_show_netlink_file(struct filter *f)
 {
 	FILE	*fp;
 	char	buf[16384];
+	int	err = -1;
 
 	if ((fp = fopen(getenv("TCPDIAG_FILE"), "r")) == NULL) {
 		perror("fopen($TCPDIAG_FILE)");
-		return -1;
+		return err;
 	}
 
 	while (1) {
-		int status, err;
+		int status, err2;
 		struct nlmsghdr *h = (struct nlmsghdr *)buf;
 		struct sockstat s = {};
 
 		status = fread(buf, 1, sizeof(*h), fp);
 		if (status < 0) {
 			perror("Reading header from $TCPDIAG_FILE");
-			return -1;
+			break;
 		}
 		if (status != sizeof(*h)) {
 			perror("Unexpected EOF reading $TCPDIAG_FILE");
-			return -1;
+			break;
 		}
 
 		status = fread(h+1, 1, NLMSG_ALIGN(h->nlmsg_len-sizeof(*h)), fp);
 
 		if (status < 0) {
 			perror("Reading $TCPDIAG_FILE");
-			return -1;
+			break;
 		}
 		if (status + sizeof(*h) < h->nlmsg_len) {
 			perror("Unexpected EOF reading $TCPDIAG_FILE");
-			return -1;
+			break;
 		}
 
 		/* The only legal exit point */
-		if (h->nlmsg_type == NLMSG_DONE)
-			return 0;
+		if (h->nlmsg_type == NLMSG_DONE) {
+			err = 0;
+			break;
+		}
 
 		if (h->nlmsg_type == NLMSG_ERROR) {
 			struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(h);
@@ -2732,7 +2735,7 @@ static int tcp_show_netlink_file(struct filter *f)
 				errno = -err->error;
 				perror("TCPDIAG answered");
 			}
-			return -1;
+			break;
 		}
 
 		parse_diag_msg(h, &s);
@@ -2741,10 +2744,15 @@ static int tcp_show_netlink_file(struct filter *f)
 		if (f && f->f && run_ssfilter(f->f, &s) == 0)
 			continue;
 
-		err = inet_show_sock(h, &s);
-		if (err < 0)
-			return err;
+		err2 = inet_show_sock(h, &s);
+		if (err2 < 0) {
+			err = err2;
+			break;
+		}
 	}
+
+	fclose(fp);
+	return err;
 }
 
 static int tcp_show(struct filter *f, int socktype)
-- 
2.13.1

[iproute PATCH v2 2/2] tc/em_ipset: Don't leak sockfd on error path

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/em_ipset.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tc/em_ipset.c b/tc/em_ipset.c
index fab975f5ea563..b59756515d239 100644
--- a/tc/em_ipset.c
+++ b/tc/em_ipset.c
@@ -84,6 +84,7 @@ static int get_version(unsigned int *version)
 	res = getsockopt(sockfd, SOL_IP, SO_IP_SET, &req_version, &size);
 	if (res != 0) {
 		perror("xt_set getsockopt");
+		close(sockfd);
 		return -1;
 	}
 
-- 
2.13.1

Re: [iproute PATCH v2 0/2] Covscan: Fix potential file descriptor leaks

[Stephen Hemminger]

On Thu, 17 Aug 2017 19:09:29 +0200
Phil Sutter <phil@nwl.cc> wrote:

> This series collects patches from v1 which deal with potential file
> descriptor leaks.
> 
> No changes to the actual patches, just splitting into smaller series.
> 
> Phil Sutter (2):
>   ss: Don't leak fd in tcp_show_netlink_file()
>   tc/em_ipset: Don't leak sockfd on error path
> 
>  misc/ss.c     | 32 ++++++++++++++++++++------------
>  tc/em_ipset.c |  1 +
>  2 files changed, 21 insertions(+), 12 deletions(-)
> 

Applied