Re: [PATCH 1/7] cleanups: Fix resource leak and buffer overrun in daemon.c

From: Serge E. Hallyn <serue@us.ibm.com>
Date: Apr 18, 2006, 13:11 UTC
Message-ID: <20060418131106.GD7562@sergelap.austin.ibm.com>
In-Reply-To: <7vu08rkhey.fsf@assigned-by-dhcp.cox.net>

Quoting Junio C Hamano (junkio@cox.net):

Show 21 quoted lines

> "Serge E. Hallyn" <serue@us.ibm.com> writes:
> 
> > Address two reports from an automatic code analyzer:
> >
> > 1. In logreport, it is possible to write \0 one
> > character past the end of buf[].
> 
> I am perhaps slower than I usually am today, but it seems to me
> that the code caps msglen to (maxlen-1) and then adds that to
> buflen.
> 
> Now, maxlen is (sizeof(buf)-buflen-1), so that means after
> the "buflen += msglen" happens, buflen is at most:
> 
> 	buflen + (sizeof(buf)-buflen-1) - 1
>         = sizeof(buf) - 2
> 
> And then "buf[buflen++] = '\n'; buf[buflen] = '\0'" happens.
> '\n' is written at sizeof(buf)-2 (or lower index than that) and
> '\0' is written at sizeof(buf)-1 (or lower).  I am unsure how it
> steps beyond the end...

Argh, I had to pull out a sheet of paper, but you are right. I misread, and the warning must be about the case where the snprint "[%ld] " prints out 1023 characters.

Show 6 quoted lines

> > 2. In socksetup, socklist can be leaked when returning
> > if set_reuse_addr().  Note: dunno why this case returns...
> 
> I am not sure why this part returns either.  It appears to me
> that it should just keep going just like the cases where
> bind/listen fails.

Then perhaps the following is more appropriate.

thanks, -serge

From: Serge E. Hallyn <serue@us.ibm.com>
Subject: [PATCH] socksetup: don't return on set_reuse_addr() error

The set_reuse_addr() error case was the only error case in socklist() where we returned rather than continued. Not sure why. Either we must free the socklist, or continue. This patch continues on error.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>

---

 daemon.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

b589029e3187eed51c3fe6a2715f51bea2159786

diff --git a/daemon.c b/daemon.c
index a1ccda3..776749e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -535,7 +535,7 @@ static int socksetup(int port, int **soc
 
 		if (set_reuse_addr(sockfd)) {
 			close(sockfd);
-			return 0;	/* not fatal */
+			continue;
 		}
 
 		if (bind(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {

-- 
1.2.5