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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Junio C Hamano (junkio@xxxxxxx):
> "Serge E. Hallyn" <serue@xxxxxxxxxx> 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@xxxxxxxxxx>
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@xxxxxxxxxx>

---

 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

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]