NetBSD Problem Report #49247

From taca@back-street.net  Tue Sep 30 08:31:01 2014
Return-Path: <taca@back-street.net>
Received: from mail.netbsd.org (mail.netbsd.org [149.20.53.66])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
	by mollari.NetBSD.org (Postfix) with ESMTPS id C52E4A6552
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 30 Sep 2014 08:31:01 +0000 (UTC)
Message-Id: <20140930083046.44A94B097@edge.back-street.net>
Date: Tue, 30 Sep 2014 17:30:46 +0900 (JST)
From: taca@back-street.net
Reply-To: taca@back-street.net
To: gnats-bugs@NetBSD.org
Subject: getsockname(2) requires sockaddr structure to cleared
X-Send-Pr-Version: 3.95

>Number:         49247
>Category:       kern
>Synopsis:       getsockname(2) requires sockaddr structure to cleared
>Confidential:   no
>Severity:       serious
>Priority:       high
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Sep 30 08:35:00 +0000 2014
>Closed-Date:    Tue Oct 14 05:50:33 +0000 2014
>Last-Modified:  Tue Oct 14 05:50:33 +0000 2014
>Originator:     Takahiro Kambe
>Release:        NetBSD 7.0_BETA
>Organization:
Takahiro Kambe
>Environment:


System: NetBSD edge.back-street.net 7.0_BETA NetBSD 7.0_BETA (VMWARE-F6) #2: Sat Sep 13 08:53:11 JST 2014 taca@edge.back-street.net:/data/amd64/obj/sys/arch/amd64/compile/VMWARE-F6 amd64
Architecture: x86_64
Machine: amd64
>Description:
	getsockname(2) requires sockaddr structure to zero cleared when
	passing UNIX domain socket.  On NetBSD 6.1_STABLE, no such
	assumeption was needed.

	And it break lmtp support of dovecot2.

>How-To-Repeat:
	Here is a simple sample program.  Executing without argument,
	storange output on NetBSD 7.0_BETA.

% ./a.out
18: /var/tmp/socktest0aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
18: /var/tmp/socktest1aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa

	With some argument or execute on NetBSD 6.1_STABLE,

% ./a.out foo
18: /var/tmp/socktest0
18: /var/tmp/socktest1

#include <stdio.h>
#include <stdlib.h>
#include <stddef.h>
#include <string.h>
#include <err.h>
#include <errno.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/un.h>

#define UPATH	"/var/tmp/socktest"
#define N	2

int
make_sock(char *path)
{
	int fd;
	struct sockaddr_un un;

	fd = socket(PF_LOCAL, SOCK_STREAM, 0);
	if (fd < 0)
		err(EXIT_FAILURE, "socket");

	un.sun_family = AF_UNIX;
	strcpy(un.sun_path, path);
	un.sun_len = SUN_LEN(&un);

	(void)unlink(path);
	if (bind(fd, (struct sockaddr *)&un, sizeof un))
		err(EXIT_FAILURE, "bind");
	return fd;
}

int
main(int argc, char *argv[])
{
	int i, n, clear, fd[N];
	struct sockaddr_un *run;
	struct sockaddr_storage ss;
	socklen_t len;
	char buf[BUFSIZ];

	clear = (argc > 1)? 1: 0;

	n = N;
	for (i = 0; i < n; i++) {
		sprintf(buf, "%s%d", UPATH, i);
		fd[i] = make_sock(buf);
	}

	for (i = 0; i < n; i++) {
		if (clear)
			memset(&ss, 0, sizeof ss);
		else
			memset(&ss, 'a', sizeof ss);
		run = (struct sockaddr_un *)&ss;
		len = sizeof *run;

		if (getsockname(fd[i], (struct sockaddr *)run, &len))
			err(EXIT_FAILURE, "getsockname");

		printf("%d: %s\n", len - 2, run->sun_path);
	}
	return EXIT_SUCCESS;
}

>Fix:
	Unknown.  Workaround is make user program to clear sockaddr
	structure before calling getsockname(2).

	And I don't check in the case of getpeername(2).

>Release-Note:

>Audit-Trail:
From: Joerg Sonnenberger <joerg@britannica.bec.de>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/49247: getsockname(2) requires sockaddr structure to cleared
Date: Tue, 30 Sep 2014 11:18:03 +0200

 --9amGYk9869ThD9tj
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline

 Please try the attached patch.

 Joerg

 --9amGYk9869ThD9tj
 Content-Type: text/x-diff; charset=us-ascii
 Content-Disposition: attachment; filename="uipc_usrreq.c.diff"

 Index: uipc_usrreq.c
 ===================================================================
 RCS file: /home/joerg/repo/netbsd/src/sys/kern/uipc_usrreq.c,v
 retrieving revision 1.171
 diff -u -p -r1.171 uipc_usrreq.c
 --- uipc_usrreq.c	5 Sep 2014 09:20:59 -0000	1.171
 +++ uipc_usrreq.c	30 Sep 2014 09:14:31 -0000
 @@ -354,7 +354,7 @@ unp_setaddr(struct socket *so, struct mb
  		}
  		if (sun == NULL)
  			sun = &sun_noname;
 -		nam->m_len = sun->sun_len;
 +		nam->m_len = sun->sun_len + 1;
  		if (nam->m_len > MLEN && !ext) {
  			sounlock(so);
  			MEXTMALLOC(nam, MAXPATHLEN * 2, M_WAITOK);
 @@ -362,7 +362,8 @@ unp_setaddr(struct socket *so, struct mb
  			ext = true;
  		} else {
  			KASSERT(nam->m_len <= MAXPATHLEN * 2);
 -			memcpy(mtod(nam, void *), sun, (size_t)nam->m_len);
 +			memcpy(mtod(nam, void *), sun, (size_t)nam->m_len - 1);
 +			mtod(nam, char *)[nam->m_len] = 0;
  			break;
  		}
  	}

 --9amGYk9869ThD9tj--

From: Takahiro Kambe <taca@back-street.net>
To: gnats-bugs@NetBSD.org, joerg@britannica.bec.de
Cc: 
Subject: Re: kern/49247: getsockname(2) requires sockaddr structure to
 cleared
Date: Tue, 30 Sep 2014 18:34:12 +0900 (JST)

 In message <20140930092000.E75B5A65C2@mollari.NetBSD.org>
 	on Tue, 30 Sep 2014 09:20:00 +0000 (UTC),
 	Joerg Sonnenberger <joerg@britannica.bec.de> wrote:
 >  Please try the attached patch.
 >  
 >  Joerg
 Thanks.  Sure, it is fixed.

 -- 
 Takahiro Kambe <taca@back-street.net>

From: Takahiro Kambe <taca@back-street.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/49247: getsockname(2) requires sockaddr structure to
 cleared
Date: Fri, 03 Oct 2014 10:58:46 +0900 (JST)

 In message <20140930.183412.842932544742766590.taca@back-street.net>
 	on Tue, 30 Sep 2014 18:34:12 +0900 (JST),
 	Takahiro Kambe <taca@back-street.net> wrote:
 > In message <20140930092000.E75B5A65C2@mollari.NetBSD.org>
 > 	on Tue, 30 Sep 2014 09:20:00 +0000 (UTC),
 > 	Joerg Sonnenberger <joerg@britannica.bec.de> wrote:
 >>  Please try the attached patch.
 >>  
 >>  Joerg
 > Thanks.  Sure, it is fixed.
 Joerg's fix solve the problem but I wonder why it was no problem on
 NetBSD 6.1_STABLE.

 After checking the result of the test program and I'm sure below
 change is correct and make the behavior as the same as 6.1_STABLE.

 (The change is in makeun().)

 Index: sys/kern/uipc_usrreq.c
 ===================================================================
 RCS file: /cvsroot/src/sys/kern/uipc_usrreq.c,v
 retrieving revision 1.171
 diff -u -r1.171 uipc_usrreq.c
 --- sys/kern/uipc_usrreq.c	5 Sep 2014 09:20:59 -0000	1.171
 +++ sys/kern/uipc_usrreq.c	3 Oct 2014 01:54:11 -0000
 @@ -927,8 +927,6 @@
  	sun = malloc(*addrlen, M_SONAME, M_WAITOK);
  	m_copydata(nam, 0, nam->m_len, (void *)sun);
  	*(((char *)sun) + nam->m_len) = '\0';
 -	sun->sun_len = strlen(sun->sun_path) +
 -	    offsetof(struct sockaddr_un, sun_path);
  	return sun;
  }


 -- 
 Takahiro Kambe <taca@back-street.net>

From: Takahiro Kambe <taca@back-street.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: kern/49247: getsockname(2) requires sockaddr structure to
 cleared
Date: Fri, 03 Oct 2014 13:46:46 +0900 (JST)

 In message <20141003.105846.102792880237070983.taca@back-street.net>
 	on Fri, 03 Oct 2014 10:58:46 +0900 (JST),
 	Takahiro Kambe <taca@back-street.net> wrote:
 > After checking the result of the test program and I'm sure below
 > change is correct and make the behavior as the same as 6.1_STABLE.
 > 
 > (The change is in makeun().)
 Attached patch was against NetBSD current.

 -- 
 Takahiro Kambe <taca@back-street.net>

From: "Takahiro Kambe" <taca@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49247 CVS commit: src/sys/kern
Date: Wed, 8 Oct 2014 16:13:03 +0000

 Module Name:	src
 Committed By:	taca
 Date:		Wed Oct  8 16:13:02 UTC 2014

 Modified Files:
 	src/sys/kern: uipc_usrreq.c

 Log Message:
 Make behavior of getsockname(2) (and maybe getpeername(2)) as the same as
 NetBSD 6.1_STABLE and other operating system (OS X 10.9.5).

 * sa_len of sockaddr_un strucrure is always set to sizeof(sun_path).
 * pathname stored in sun_path is alwasys '\0' terminated (except length
   of sun_path is sizeof(sun_path)?).

 Should be fix PR kern/49247, runtime problem of lmtp service of dovecot2 on
 NetBSD current and NetBSD 7.0_BETA.


 To generate a diff of this commit:
 cvs rdiff -u -r1.171 -r1.172 src/sys/kern/uipc_usrreq.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: "Soren Jacobsen" <snj@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/49247 CVS commit: [netbsd-7] src/sys/kern
Date: Sat, 11 Oct 2014 16:16:44 +0000

 Module Name:	src
 Committed By:	snj
 Date:		Sat Oct 11 16:16:44 UTC 2014

 Modified Files:
 	src/sys/kern [netbsd-7]: uipc_usrreq.c

 Log Message:
 Pull up following revision(s) (requested by taca in ticket #132):
 	sys/kern/uipc_usrreq.c: revision 1.172
 Make behavior of getsockname(2) (and maybe getpeername(2)) as the same as
 NetBSD 6.1_STABLE and other operating system (OS X 10.9.5).
 * sa_len of sockaddr_un strucrure is always set to sizeof(sun_path).
 * pathname stored in sun_path is alwasys '\0' terminated (except length
   of sun_path is sizeof(sun_path)?).
 Should be fix PR kern/49247, runtime problem of lmtp service of dovecot2 on
 NetBSD current and NetBSD 7.0_BETA.


 To generate a diff of this commit:
 cvs rdiff -u -r1.169 -r1.169.2.1 src/sys/kern/uipc_usrreq.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Tue, 14 Oct 2014 05:50:33 +0000
State-Changed-Why:
fixed and pulled up, thanks


>Unformatted:

Home	PR Database Search