[PATCH] add SOCK_STREAM support for syslog

[PATCH] add SOCK_STREAM support for syslog

[Hrvoje Varga]

To be consistent with glibc and uClibc since both glibc and uClibc
have a support for connecting to SOCK_STREAM socket in case
when connection to a SOCK_DGRAM is unsuccessful. This patch mimics the
same behavior.

There was a discussion whether the musl should support the SOCK_STREAM
on musl mailing list http://www.openwall.com/lists/musl/2015/08/10/1.

Signed-off-by: Hrvoje Varga <hrvoje.varga@sartura.hr>
---
 src/misc/syslog.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/misc/syslog.c b/src/misc/syslog.c
index 9dd1ddb5..101847a4 100644
--- a/src/misc/syslog.c
+++ b/src/misc/syslog.c
@@ -48,8 +48,22 @@ void closelog(void)
 
 static void __openlog()
 {
-	log_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
-	if (log_fd >= 0) connect(log_fd, (void *)&log_addr, sizeof log_addr);
+	int retry = 0;
+	int sock_type = SOCK_DGRAM;
+	while (retry < 2) {
+		retry++;
+		log_fd = socket(AF_UNIX, sock_type|SOCK_CLOEXEC, 0);
+		if (log_fd >= 0) {
+			if (connect(log_fd, (void *)&log_addr, sizeof log_addr) != -1) {
+				return;
+			}
+			if (errno == EPROTOTYPE) {
+				sock_type = SOCK_STREAM;
+			}
+			close(log_fd);
+			log_fd = -1;
+		}
+	}
 }
 
 void openlog(const char *ident, int opt, int facility)
-- 
2.14.1

Re: [PATCH] add SOCK_STREAM support for syslog

[Hrvoje Varga]

> To be consistent with glibc and uClibc since both glibc and uClibc
> have a support for connecting to SOCK_STREAM socket in case
> when connection to a SOCK_DGRAM is unsuccessful. This patch mimics the
> same behavior.
> 
> There was a discussion whether the musl should support the SOCK_STREAM
> on musl mailing list http://www.openwall.com/lists/musl/2015/08/10/1.
> 
> Signed-off-by: Hrvoje Varga <hrvoje.varga@sartura.hr>
> ---
>  src/misc/syslog.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/src/misc/syslog.c b/src/misc/syslog.c
> index 9dd1ddb5..101847a4 100644
> --- a/src/misc/syslog.c
> +++ b/src/misc/syslog.c
> @@ -48,8 +48,22 @@ void closelog(void)
>  
>  static void __openlog()
>  {
> -	log_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
> -	if (log_fd >= 0) connect(log_fd, (void *)&log_addr, sizeof log_addr);
> +	int retry = 0;
> +	int sock_type = SOCK_DGRAM;
> +	while (retry < 2) {
> +		retry++;
> +		log_fd = socket(AF_UNIX, sock_type|SOCK_CLOEXEC, 0);
> +		if (log_fd >= 0) {
> +			if (connect(log_fd, (void *)&log_addr, sizeof log_addr) != -1) {
> +				return;
> +			}
> +			if (errno == EPROTOTYPE) {
> +				sock_type = SOCK_STREAM;
> +			}
> +			close(log_fd);
> +			log_fd = -1;
> +		}
> +	}
>  }
>  
>  void openlog(const char *ident, int opt, int facility)
> -- 
> 2.14.1
> 

Hi,

any news regarding this patch?

-- 
br,
Hrvoje

Re: Re: [PATCH] add SOCK_STREAM support for syslog

[Rich Felker]

On Thu, Apr 12, 2018 at 03:04:31PM +0200, Hrvoje Varga wrote:
> > To be consistent with glibc and uClibc since both glibc and uClibc
> > have a support for connecting to SOCK_STREAM socket in case
> > when connection to a SOCK_DGRAM is unsuccessful. This patch mimics the
> > same behavior.
> > 
> > There was a discussion whether the musl should support the SOCK_STREAM
> > on musl mailing list http://www.openwall.com/lists/musl/2015/08/10/1.

Yes, and it never really concluded. I think it's probably harmless to
support SOCK_STREAM if users want it though. But the patch is probably
insufficient.

One question I had that never got answered: are SOCK_DGRAM and
SOCK_STREAM unix socket addresses mutually exclusive, or can the same
name be listening as both types? This affects how we handle certain
error cases I think..

> > Signed-off-by: Hrvoje Varga <hrvoje.varga@sartura.hr>
> > ---
> >  src/misc/syslog.c | 18 ++++++++++++++++--
> >  1 file changed, 16 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/misc/syslog.c b/src/misc/syslog.c
> > index 9dd1ddb5..101847a4 100644
> > --- a/src/misc/syslog.c
> > +++ b/src/misc/syslog.c
> > @@ -48,8 +48,22 @@ void closelog(void)
> >  
> >  static void __openlog()
> >  {
> > -	log_fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
> > -	if (log_fd >= 0) connect(log_fd, (void *)&log_addr, sizeof log_addr);
> > +	int retry = 0;
> > +	int sock_type = SOCK_DGRAM;
> > +	while (retry < 2) {
> > +		retry++;
> > +		log_fd = socket(AF_UNIX, sock_type|SOCK_CLOEXEC, 0);
> > +		if (log_fd >= 0) {
> > +			if (connect(log_fd, (void *)&log_addr, sizeof log_addr) != -1) {
> > +				return;
> > +			}
> > +			if (errno == EPROTOTYPE) {
> > +				sock_type = SOCK_STREAM;
> > +			}
> > +			close(log_fd);
> > +			log_fd = -1;
> > +		}
> > +	}
> >  }
> >  
> >  void openlog(const char *ident, int opt, int facility)

This only covers opening, not reconnection on loss, which happens in
_vsyslog. The way reconnection would have to be handled for
SOCK_STREAM is different and less robust, I think, so we probably
can't just replace the existing code but would need a new code path
for it.

Rich

Re: Re: [PATCH] add SOCK_STREAM support for syslog

[Laurent Bercot]

>Yes, and it never really concluded. I think it's probably harmless to
>support SOCK_STREAM if users want it though.

  I do want it. It's the first step towards a simple, non-bloated
implementation of syslogd. Please support SOCK_STREAM whenever there's
nothing more urgent to do. :P


>One question I had that never got answered: are SOCK_DGRAM and
>SOCK_STREAM unix socket addresses mutually exclusive, or can the same
>name be listening as both types?

  They appear to be mutually exclusive.

execve("./test-socket", ["./test-socket"], [/* 24 vars */]) = 0
arch_prctl(ARCH_SET_FS, 0x602538) = 0
set_tid_address(0x602570) = 9828
socket(PF_LOCAL, SOCK_STREAM|SOCK_NONBLOCK, 0) = 3
setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
socket(PF_LOCAL, SOCK_DGRAM|SOCK_NONBLOCK, 0) = 4
setsockopt(4, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
unlink("/tmp/socket") = 0
bind(3, {sa_family=AF_LOCAL, sun_path="/tmp/socket"}, 110) = 0
bind(4, {sa_family=AF_LOCAL, sun_path="/tmp/socket"}, 110) = -1 
EADDRINUSE (Address already in use)

--
  Laurent