Subject: Re: cleaning up the rest of dtom()
To: enami tsugutomo <enami@ba2.so-net.or.jp>
From: Jason Thorpe <thorpej@nas.nasa.gov>
List: tech-kern
Date: 06/25/1997 12:35:53
On 25 Jun 1997 21:08:20 +0900
enami tsugutomo <enami@ba2.so-net.or.jp> wrote:
> Does this patch intend to unlimit the length of unix domain socket
> address rather than limits to sizeof (sockaddr_un) or MLEN?
>
> Hmm..., then, I think:
>
> * unp_connect() also has similar check for the address if it just fit
> to mbuf. This also can be simplified like unp_bind().
>
> * unp_setsockaddr() and unp_setpeeraddr() are using bcopy() to copy
> unp->unp_addr to mbuf. It may overruns.
>
> * sbappendaddr() called via unp_output() limits asa->sa_len (is
> sun_len) to MLEN.
>
> How about?
Ok, the patch below should cover all of these cases. Comments?
Jason R. Thorpe thorpej@nas.nasa.gov
NASA Ames Research Center Home: 408.866.1912
NAS: M/S 258-6 Work: 415.604.0935
Moffett Field, CA 94035 Pager: 415.428.6939
Index: uipc_socket2.c
===================================================================
RCS file: /mastersrc/netbsd/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.14
diff -c -r1.14 uipc_socket2.c
*** uipc_socket2.c 1997/01/13 17:36:13 1.14
--- uipc_socket2.c 1997/06/25 18:53:36
***************
*** 598,608 ****
}
if (space > sbspace(sb))
return (0);
- if (asa->sa_len > MLEN)
- return (0);
MGET(m, M_DONTWAIT, MT_SONAME);
if (m == 0)
return (0);
m->m_len = asa->sa_len;
bcopy((caddr_t)asa, mtod(m, caddr_t), asa->sa_len);
if (n)
--- 598,613 ----
}
if (space > sbspace(sb))
return (0);
MGET(m, M_DONTWAIT, MT_SONAME);
if (m == 0)
return (0);
+ if (asa->sa_len > MLEN) {
+ MEXTMALLOC(m, asa->sa_len, M_NOWAIT);
+ if ((m->m_flags & M_EXT) == 0) {
+ m_free(m);
+ return (0);
+ }
+ }
m->m_len = asa->sa_len;
bcopy((caddr_t)asa, mtod(m, caddr_t), asa->sa_len);
if (n)
Index: uipc_syscalls.c
===================================================================
RCS file: /mastersrc/netbsd/src/sys/kern/uipc_syscalls.c,v
retrieving revision 1.1.1.6
diff -c -r1.1.1.6 uipc_syscalls.c
*** uipc_syscalls.c 1997/01/07 01:20:13 1.1.1.6
--- uipc_syscalls.c 1997/06/25 16:36:24
***************
*** 968,973 ****
--- 968,977 ----
return (error);
}
+ /*
+ * XXX In a perfect world, we wouldn't pass around socket control
+ * XXX arguments in mbufs, and this could go away.
+ */
int
sockargs(mp, buf, buflen, type)
struct mbuf **mp;
***************
*** 978,992 ****
register struct mbuf *m;
int error;
if ((u_int)buflen > MLEN) {
! #ifdef COMPAT_OLDSOCK
! if (type == MT_SONAME && (u_int)buflen <= 112)
! buflen = MLEN; /* unix domain compat. hack */
! else
! #endif
! return (EINVAL);
}
- m = m_get(M_WAIT, type);
m->m_len = buflen;
error = copyin(buf, mtod(m, caddr_t), (u_int)buflen);
if (error) {
--- 982,996 ----
register struct mbuf *m;
int error;
+ /* Allocate an mbuf to hold the arguments. */
+ m = m_get(M_WAIT, type);
if ((u_int)buflen > MLEN) {
! /*
! * Won't fit into a regular mbuf, so we allocate just
! * enough external storage to hold the argument.
! */
! MEXTMALLOC(m, buflen, M_WAITOK);
}
m->m_len = buflen;
error = copyin(buf, mtod(m, caddr_t), (u_int)buflen);
if (error) {
Index: uipc_usrreq.c
===================================================================
RCS file: /mastersrc/netbsd/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.1.1.7
diff -c -r1.1.1.7 uipc_usrreq.c
*** uipc_usrreq.c 1997/06/24 21:51:44 1.1.1.7
--- uipc_usrreq.c 1997/06/25 18:52:36
***************
*** 99,104 ****
--- 99,106 ----
else
sun = &sun_noname;
nam->m_len = sun->sun_len;
+ if (nam->m_len > MLEN)
+ MEXTMALLOC(nam, nam->m_len, M_WAITOK);
bcopy(sun, mtod(nam, caddr_t), (size_t)nam->m_len);
}
***************
*** 114,119 ****
--- 116,123 ----
else
sun = &sun_noname;
nam->m_len = sun->sun_len;
+ if (nam->m_len > MLEN)
+ MEXTMALLOC(nam, nam->m_len, M_WAITOK);
bcopy(sun, mtod(nam, caddr_t), (size_t)nam->m_len);
}
***************
*** 413,438 ****
struct mbuf *nam;
struct proc *p;
{
! struct sockaddr_un *sun = mtod(nam, struct sockaddr_un *);
register struct vnode *vp;
struct vattr vattr;
int error;
struct nameidata nd;
- if (nam->m_len > sizeof(struct sockaddr_un))
- return (EINVAL);
if (unp->unp_vnode != 0)
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);
vp = nd.ni_vp;
if (vp != NULL) {
VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd);
--- 417,448 ----
struct mbuf *nam;
struct proc *p;
{
! struct sockaddr_un *sun;
register struct vnode *vp;
struct vattr vattr;
+ size_t addrlen;
int error;
struct nameidata nd;
if (unp->unp_vnode != 0)
return (EINVAL);
+
+ /*
+ * Allocate the new sockaddr. We have to allocate one
+ * extra byte so that we can ensure that the pathname
+ * is nul-terminated.
+ */
+ addrlen = nam->m_len + 1;
+ sun = malloc(addrlen, M_SONAME, M_WAITOK);
+ m_copydata(nam, 0, nam->m_len, (caddr_t)sun);
+ *(((char *)sun) + nam->m_len) = '\0';
+
NDINIT(&nd, CREATE, FOLLOW | LOCKPARENT, UIO_SYSSPACE,
sun->sun_path, p);
!
/* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
if ((error = namei(&nd)) != 0)
! goto bad;
vp = nd.ni_vp;
if (vp != NULL) {
VOP_ABORTOP(nd.ni_dvp, &nd.ni_cnd);
***************
*** 441,447 ****
else
vput(nd.ni_dvp);
vrele(vp);
! return (EADDRINUSE);
}
VATTR_NULL(&vattr);
vattr.va_type = VSOCK;
--- 451,458 ----
else
vput(nd.ni_dvp);
vrele(vp);
! error = EADDRINUSE;
! goto bad;
}
VATTR_NULL(&vattr);
vattr.va_type = VSOCK;
***************
*** 449,463 ****
VOP_LEASE(nd.ni_dvp, p, p->p_ucred, LEASE_WRITE);
error = VOP_CREATE(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, &vattr);
if (error)
! return (error);
vp = nd.ni_vp;
vp->v_socket = unp->unp_socket;
unp->unp_vnode = vp;
! unp->unp_addrlen = nam->m_len;
! unp->unp_addr = malloc(unp->unp_addrlen, M_SONAME, M_WAITOK);
! m_copydata(nam, 0, unp->unp_addrlen, (caddr_t)unp->unp_addr);
VOP_UNLOCK(vp);
return (0);
}
int
--- 460,477 ----
VOP_LEASE(nd.ni_dvp, p, p->p_ucred, LEASE_WRITE);
error = VOP_CREATE(nd.ni_dvp, &nd.ni_vp, &nd.ni_cnd, &vattr);
if (error)
! goto bad;
vp = nd.ni_vp;
vp->v_socket = unp->unp_socket;
unp->unp_vnode = vp;
! unp->unp_addrlen = addrlen;
! unp->unp_addr = sun;
VOP_UNLOCK(vp);
return (0);
+
+ bad:
+ free(sun, M_SONAME);
+ return (error);
}
int
***************
*** 466,486 ****
struct mbuf *nam;
struct proc *p;
{
! register struct sockaddr_un *sun = mtod(nam, struct sockaddr_un *);
register struct vnode *vp;
register struct socket *so2, *so3;
struct unpcb *unp2, *unp3;
int error;
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;
if (vp->v_type != VSOCK) {
error = ENOTSOCK;
--- 480,508 ----
struct mbuf *nam;
struct proc *p;
{
! register struct sockaddr_un *sun;
register struct vnode *vp;
register struct socket *so2, *so3;
struct unpcb *unp2, *unp3;
+ size_t addrlen;
int error;
struct nameidata nd;
+ /*
+ * Allocate a temporary sockaddr. We have to allocate one extra
+ * byte so that we can ensure that the pathname is nul-terminated.
+ * When we establish the connection, we copy the other PCB's
+ * sockaddr to our own.
+ */
+ addrlen = nam->m_len + 1;
+ sun = malloc(addrlen, M_SONAME, M_WAITOK);
+ m_copydata(nam, 0, nam->m_len, (caddr_t)sun);
+ *(((char *)sun) + nam->m_len) = '\0';
+
NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF, UIO_SYSSPACE, sun->sun_path, p);
!
if ((error = namei(&nd)) != 0)
! goto bad2;
vp = nd.ni_vp;
if (vp->v_type != VSOCK) {
error = ENOTSOCK;
***************
*** 515,522 ****
so2 = so3;
}
error = unp_connect2(so, so2);
! bad:
vput(vp);
return (error);
}
--- 537,546 ----
so2 = so3;
}
error = unp_connect2(so, so2);
! bad:
vput(vp);
+ bad2:
+ free(sun, M_SONAME);
return (error);
}