>Number:         3035
>Category:       kern
>Synopsis:       inconsitency about terminating null byte of UNIX domain socket name
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people (Kernel Bug People)
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Dec 15 23:20:01 1996
>Last-Modified:
>Originator:     enami tsugutomo
>Organization:
	an individual
>Release:        NetBSD-current as of 1996 Dec.
>Environment:
System: NetBSD pavlov.enami.ba2.so-net.or.jp 1.2B NetBSD 1.2B (PAVLOV) #125: Fri Dec 13 14:06:50 JST 1996 enami@pavlov.enami.ba2.so-net.or.jp:/b/netbsd/kernel/compile/PAVLOV i386


>Description:
	There is an inconsitency about terminating null byte of UNIX domain
	socket name.

	1. In sys/un.h, the comment of member `sun_len' of struct sockaddr_un
	says that it includes terminating null byte.  But in the kernel, that
	is initalized just given `namelen' which is third argument of bind(2).

	It means that whether `sun_len' include the terminating null byte or
	not just depends on how userland program specifies.

	Especially, if userland uses macro SUN_LEN (defined in sys/un.h) to
	determine the third argument for bind(2), `sun_len' doesn't include
	the terminating null byte.

	2. When the netstat(1) prints UNIX domain socket name, it expect that
	the sun_path field (in the copy of socket name) to be terminated by
	null byte.  But whether it is terminated by null or not depends on how
	userland program specifies the address length (quite similar to above).

	(and I noticed that, in the sample code of the book "Advanced
	Programming in Unix Environment, the third argument of bind(2)
	is coded to includes the terminating null byte if Reno or after).
	
>How-To-Repeat:
	For both 1. and 2., please read the source, especially sys/un.h and
	kern/uipc_usrreq.c.

	For 2., I can get following output from netstat | cat -vet | grep
	/dev/log on my machine.  There is junk at the end of pathname.

f88c6100 dgram       0      0 f88c8700        0 f8926580        0 /dev/logM--M-^$
>Fix:
	Here is a patch to:
		* make SUN_LEN to include terminating null byte.
		* make sun_len and sun_path (in the copy of address) always
		includes terminating null byte.  (this is for backward
		compatibility).
		* reject too short names for UNIX domain sockets explicitly.

Index: kern/uipc_usrreq.c
===================================================================
RCS file: /a/cvsroot/NetBSD/sys/kern/uipc_usrreq.c,v
retrieving revision 1.1.1.3
diff -c -r1.1.1.3 uipc_usrreq.c
*** uipc_usrreq.c	1996/11/16 16:24:57	1.1.1.3
--- uipc_usrreq.c	1996/12/16 06:07:43
***************
*** 416,426 ****
  		return (EINVAL);
  	NDINIT(&nd, CREATE, FOLLOW | LOCKPARENT, UIO_SYSSPACE,
  	    sun->sun_path, p);
! 	if (nam->m_data + nam->m_len == &nam->m_dat[MLEN]) {	/* XXX */
! 		if (*(mtod(nam, caddr_t) + nam->m_len - 1) != 0)
  			return (EINVAL);
! 	} else
! 		*(mtod(nam, caddr_t) + nam->m_len) = 0;
  /* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
  	if ((error = namei(&nd)) != 0)
  		return (error);
--- 416,432 ----
  		return (EINVAL);
  	NDINIT(&nd, CREATE, FOLLOW | LOCKPARENT, UIO_SYSSPACE,
  	    sun->sun_path, p);
! 	if (nam->m_len <= 2)	/* too short as UNIX domain socket name. */
! 		return (EINVAL);
! 	else if (*(mtod(nam, caddr_t) + nam->m_len - 1) != 0) {
! 		if (nam->m_data + nam->m_len == &nam->m_dat[MLEN]) /* XXX */
  			return (EINVAL);
! 		else {
! 			*(mtod(nam, caddr_t) + nam->m_len) = 0;
! 			nam->m_len++;
! 			sun->sun_len++;
! 		}
! 	}
  /* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
  	if ((error = namei(&nd)) != 0)
  		return (error);
***************
*** 464,474 ****
  	struct nameidata nd;
  
  	NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, sun->sun_path, p);
! 	if (nam->m_data + nam->m_len == &nam->m_dat[MLEN]) {	/* XXX */
! 		if (*(mtod(nam, caddr_t) + nam->m_len - 1) != 0)
  			return (EINVAL);
! 	} else
! 		*(mtod(nam, caddr_t) + nam->m_len) = 0;
  	if ((error = namei(&nd)) != 0)
  		return (error);
  	vp = nd.ni_vp;
--- 470,486 ----
  	struct nameidata nd;
  
  	NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, sun->sun_path, p);
! 	if (nam->m_len <= 2)	/* too short as UNIX domain socket name. */
! 		return (EINVAL);
! 	else if (*(mtod(nam, caddr_t) + nam->m_len - 1) != 0) {
! 		if (nam->m_data + nam->m_len == &nam->m_dat[MLEN]) /* XXX */
  			return (EINVAL);
! 		else {
! 			*(mtod(nam, caddr_t) + nam->m_len) = 0;
! 			nam->m_len++;
! 			sun->sun_len++;
! 		}
! 	}
  	if ((error = namei(&nd)) != 0)
  		return (error);
  	vp = nd.ni_vp;
Index: sys/un.h
===================================================================
RCS file: /a/cvsroot/NetBSD/sys/sys/un.h,v
retrieving revision 1.1.1.3
diff -c -r1.1.1.3 un.h
*** un.h	1996/11/16 16:29:40	1.1.1.3
--- un.h	1996/12/16 06:21:32
***************
*** 70,74 ****
  
  /* actual length of an initialized sockaddr_un */
  #define SUN_LEN(su) \
! 	(sizeof(*(su)) - sizeof((su)->sun_path) + strlen((su)->sun_path))
  #endif /* _KERNEL */
--- 70,74 ----
  
  /* actual length of an initialized sockaddr_un */
  #define SUN_LEN(su) \
! 	(sizeof(*(su)) - sizeof((su)->sun_path) + strlen((su)->sun_path) + 1)
  #endif /* _KERNEL */
>Audit-Trail:
>Unformatted: