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

Quoting Junio C Hamano (junkio@cox.net):
> "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.

> > 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

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html