Fix PR kern/185813 "SOCK_SEQPACKET AF_UNIX sockets with asymmetrical

buffers drop packets".  It was caused by a check for the space available
in a sockbuf, but it was checking the wrong sockbuf.

sys/sys/sockbuf.h
sys/kern/uipc_sockbuf.c
    Add sbappendaddr_nospacecheck_locked(), which is just like
    sbappendaddr_locked but doesn't validate the receiving socket's
    space.  Factor out common code into sbappendaddr_locked_internal().
    We shouldn't simply make sbappendaddr_locked check the space and
    then call sbappendaddr_nospacecheck_locked, because that would cause
    the O(n) function m_length to be called twice.

sys/kern/uipc_usrreq.c
    Use sbappendaddr_nospacecheck_locked for SOCK_SEQPACKET sockets,
    because the receiving sockbuf's size limit is irrelevant.

tests/sys/kern/unix_seqpacket_test.c
    Now that 185813 is fixed, pipe_128k_8k fails intermittently due to
    185812.  Make it fail every time by adding a usleep after starting
    the writer thread and before starting the reader thread in
    test_pipe.  That gives the writer time to fill up its send buffer.
    Also, clear the expected failure message due to 185813.  It actually
    said "185812", but that was a typo.

PR:		kern/185813
Reviewed by:	silence from freebsd-net@ and rwatson@
MFC after:	3 weeks
Sponsored by:	Spectra Logic Corporation
This commit is contained in:
Alan Somers 2014-03-06 20:24:15 +00:00
parent bbd054cb33
commit 8de34a88de
4 changed files with 68 additions and 27 deletions

View File

@ -620,29 +620,12 @@ sbappendrecord(struct sockbuf *sb, struct mbuf *m0)
SOCKBUF_UNLOCK(sb); SOCKBUF_UNLOCK(sb);
} }
/* /* Helper routine that appends data, control, and address to a sockbuf. */
* Append address and data, and optionally, control (ancillary) data to the static int
* receive queue of a socket. If present, m0 must include a packet header sbappendaddr_locked_internal(struct sockbuf *sb, const struct sockaddr *asa,
* with total length. Returns 0 if no space in sockbuf or insufficient struct mbuf *m0, struct mbuf *control, struct mbuf *ctrl_last)
* mbufs.
*/
int
sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa,
struct mbuf *m0, struct mbuf *control)
{ {
struct mbuf *m, *n, *nlast; struct mbuf *m, *n, *nlast;
int space = asa->sa_len;
SOCKBUF_LOCK_ASSERT(sb);
if (m0 && (m0->m_flags & M_PKTHDR) == 0)
panic("sbappendaddr_locked");
if (m0)
space += m0->m_pkthdr.len;
space += m_length(control, &n);
if (space > sbspace(sb))
return (0);
#if MSIZE <= 256 #if MSIZE <= 256
if (asa->sa_len > MLEN) if (asa->sa_len > MLEN)
return (0); return (0);
@ -652,8 +635,8 @@ sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa,
return (0); return (0);
m->m_len = asa->sa_len; m->m_len = asa->sa_len;
bcopy(asa, mtod(m, caddr_t), asa->sa_len); bcopy(asa, mtod(m, caddr_t), asa->sa_len);
if (n) if (ctrl_last)
n->m_next = m0; /* concatenate data to control */ ctrl_last->m_next = m0; /* concatenate data to control */
else else
control = m0; control = m0;
m->m_next = control; m->m_next = control;
@ -670,6 +653,50 @@ sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa,
return (1); return (1);
} }
/*
* Append address and data, and optionally, control (ancillary) data to the
* receive queue of a socket. If present, m0 must include a packet header
* with total length. Returns 0 if no space in sockbuf or insufficient
* mbufs.
*/
int
sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa,
struct mbuf *m0, struct mbuf *control)
{
struct mbuf *ctrl_last;
int space = asa->sa_len;
SOCKBUF_LOCK_ASSERT(sb);
if (m0 && (m0->m_flags & M_PKTHDR) == 0)
panic("sbappendaddr_locked");
if (m0)
space += m0->m_pkthdr.len;
space += m_length(control, &ctrl_last);
if (space > sbspace(sb))
return (0);
return (sbappendaddr_locked_internal(sb, asa, m0, control, ctrl_last));
}
/*
* Append address and data, and optionally, control (ancillary) data to the
* receive queue of a socket. If present, m0 must include a packet header
* with total length. Returns 0 if insufficient mbufs. Does not validate space
* on the receiving sockbuf.
*/
int
sbappendaddr_nospacecheck_locked(struct sockbuf *sb, const struct sockaddr *asa,
struct mbuf *m0, struct mbuf *control)
{
struct mbuf *ctrl_last;
SOCKBUF_LOCK_ASSERT(sb);
ctrl_last = (control == NULL) ? NULL : m_last(control);
return (sbappendaddr_locked_internal(sb, asa, m0, control, ctrl_last));
}
/* /*
* Append address and data, and optionally, control (ancillary) data to the * Append address and data, and optionally, control (ancillary) data to the
* receive queue of a socket. If present, m0 must include a packet header * receive queue of a socket. If present, m0 must include a packet header

View File

@ -892,7 +892,8 @@ uipc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
from = &sun_noname; from = &sun_noname;
so2 = unp2->unp_socket; so2 = unp2->unp_socket;
SOCKBUF_LOCK(&so2->so_rcv); SOCKBUF_LOCK(&so2->so_rcv);
if (sbappendaddr_locked(&so2->so_rcv, from, m, control)) { if (sbappendaddr_nospacecheck_locked(&so2->so_rcv, from, m,
control)) {
sorwakeup_locked(so2); sorwakeup_locked(so2);
m = NULL; m = NULL;
control = NULL; control = NULL;
@ -977,8 +978,14 @@ uipc_send(struct socket *so, int flags, struct mbuf *m, struct sockaddr *nam,
const struct sockaddr *from; const struct sockaddr *from;
from = &sun_noname; from = &sun_noname;
if (sbappendaddr_locked(&so2->so_rcv, from, m, /*
control)) * Don't check for space available in so2->so_rcv.
* Unix domain sockets only check for space in the
* sending sockbuf, and that check is performed one
* level up the stack.
*/
if (sbappendaddr_nospacecheck_locked(&so2->so_rcv,
from, m, control))
control = NULL; control = NULL;
break; break;
} }

View File

@ -127,6 +127,8 @@ int sbappendaddr(struct sockbuf *sb, const struct sockaddr *asa,
struct mbuf *m0, struct mbuf *control); struct mbuf *m0, struct mbuf *control);
int sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa, int sbappendaddr_locked(struct sockbuf *sb, const struct sockaddr *asa,
struct mbuf *m0, struct mbuf *control); struct mbuf *m0, struct mbuf *control);
int sbappendaddr_nospacecheck_locked(struct sockbuf *sb,
const struct sockaddr *asa, struct mbuf *m0, struct mbuf *control);
int sbappendcontrol(struct sockbuf *sb, struct mbuf *m0, int sbappendcontrol(struct sockbuf *sb, struct mbuf *m0,
struct mbuf *control); struct mbuf *control);
int sbappendcontrol_locked(struct sockbuf *sb, struct mbuf *m0, int sbappendcontrol_locked(struct sockbuf *sb, struct mbuf *m0,

View File

@ -361,6 +361,12 @@ test_pipe(size_t sndbufsize, size_t rcvbufsize)
reader_data.so = sv[1]; reader_data.so = sv[1];
ATF_REQUIRE_EQ(0, pthread_create(&writer, NULL, test_pipe_writer, ATF_REQUIRE_EQ(0, pthread_create(&writer, NULL, test_pipe_writer,
(void*)&writer_data)); (void*)&writer_data));
/*
* Give the writer time to start writing, and hopefully block, before
* starting the reader. This increases the likelihood of the test case
* failing due to PR kern/185812
*/
usleep(1000);
ATF_REQUIRE_EQ(0, pthread_create(&reader, NULL, test_pipe_reader, ATF_REQUIRE_EQ(0, pthread_create(&reader, NULL, test_pipe_reader,
(void*)&reader_data)); (void*)&reader_data));
@ -951,7 +957,6 @@ ATF_TC_BODY(pipe_simulator_8k_128k, tc)
ATF_TC_WITHOUT_HEAD(pipe_simulator_128k_8k); ATF_TC_WITHOUT_HEAD(pipe_simulator_128k_8k);
ATF_TC_BODY(pipe_simulator_128k_8k, tc) ATF_TC_BODY(pipe_simulator_128k_8k, tc)
{ {
atf_tc_expect_fail("PR kern/185812 SOCK_SEQPACKET AF_UNIX sockets with asymmetrical buffers drop packets");
test_pipe_simulator(131072, 8192); test_pipe_simulator(131072, 8192);
} }