Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130323161755.GN19010@port70.net>
Date: Sat, 23 Mar 2013 17:17:55 +0100
From: Szabolcs Nagy <nsz@...t70.net>
To: musl@...ts.openwall.com
Subject: Re: Proposed syslog patch [Re: Further bugs in
 syslog()]

looks ok, one nitpick

* Rich Felker <dalias@...ifal.cx> [2013-03-23 00:05:58 -0400]:
>  void openlog(const char *ident, int opt, int facility)
> @@ -59,7 +54,14 @@ void openlog(const char *ident, int opt, int facility)
>  	int cs;
>  	pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
>  	LOCK(lock);
> -	__openlog(ident, opt, facility);
> +
> +	free(log_ident);
> +	log_ident = strdup(ident);

this can fail and then log_ident = 0

...
> @@ -90,14 +89,14 @@ static void _vsyslog(int priority, const char *message, va_list ap)
>  		priority, timebuf,
>  		log_ident ? log_ident : "",
>  		"["+!pid, pid, "]"+!pid);
> +	if (l >= sizeof buf) return;

if ident is longer than buf, then syslog will silently fail here,
but if ident is so big that strdup fails above it does not fail here

maybe we should limit ident to a sensible value and truncate it
(i dont know if posix allows that but seems more consistent
and then no dynamic allocation is needed in openlog)

>  	l2 = vsnprintf(buf+l, sizeof buf - l, message, ap);
>  	if (l2 >= 0) {
> -		l += l2;
> +		if (l2 >= sizeof buf - l) l = sizeof buf - 1;
> +		else l += l2;
>  		if (buf[l-1] != '\n') buf[l++] = '\n';
> -		sendto(log_fd, buf, l, 0, (void *)&log_addr, 11);
> +		send(log_fd, buf, l, 0);
>  	}
> -
> -	UNLOCK(lock);
>  }

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.