Date: Sun, 04 Sep 2011 12:30:53 +0300 From: Mikolaj Golub <trociny@freebsd.org> To: freebsd-net@freebsd.org Cc: Kostik Belousov <kib@FreeBSD.org>, Pawel Jakub Dawidek <pjd@FreeBSD.org>, Andre Oppermann <andre@FreeBSD.org> Subject: soreceive_stream: mbuf leak if called with mp0 and MSG_WAITALL Message-ID: <86ehzwwt6a.fsf@kopusha.home.net>
next in thread | raw e-mail | index | archive | help
--=-=-=
Hi,
Apparently soreceive_stream() has an issue if it is called to receive data as a
mbuf chain (by supplying an non zero mbuf **mp0) and with MSG_WAITALL set.
I ran into this issue with smbfs, which uses soreceive() exactly in this way
(see netsmb/smb_trantcp.c:nbssn_recv()).
If MSG_WAITALL is set and not all data is received it loops again but on the
next run mb0 is set to sb->sb_mb again loosing all previously received mbufs.
It looks like it should be set to the end of mb0 chain instead. See the
attached path.
Also, in the "copy the remainder" block we reduce uio_resid by m->m_len (the
length of the last mbuf in the chain), but it looks like for the MSG_PEEK case
the remainder may have more than one mbuf in the chain and we have to reduce
by len (the length of the copied chain).
I don't have a test case to check MSG_PEEK issue, but the patch fixes the
issue with smbfs for me.
--
Mikolaj Golub
--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=uipc_socket.c.soreceive_stream.patch
Index: sys/kern/uipc_socket.c
===================================================================
--- sys/kern/uipc_socket.c (revision 225368)
+++ sys/kern/uipc_socket.c (working copy)
@@ -2030,7 +2030,11 @@ deliver:
if (mp0 != NULL) {
/* Dequeue as many mbufs as possible. */
if (!(flags & MSG_PEEK) && len >= sb->sb_mb->m_len) {
- for (*mp0 = m = sb->sb_mb;
+ if (*mp0 == NULL)
+ *mp0 = sb->sb_mb;
+ else
+ n->m_next = sb->sb_mb;
+ for (m = sb->sb_mb;
m != NULL && m->m_len <= len;
m = m->m_next) {
len -= m->m_len;
@@ -2052,7 +2056,7 @@ deliver:
if (m == NULL)
len = 0; /* Don't flush data from sockbuf. */
else
- uio->uio_resid -= m->m_len;
+ uio->uio_resid -= len;
if (*mp0 != NULL)
n->m_next = m;
else
@@ -2061,6 +2065,9 @@ deliver:
error = ENOBUFS;
goto out;
}
+ /* Update n to point to the last mbuf. */
+ for (; m != NULL; m = m->m_next)
+ n = m;
}
} else {
/* NB: Must unlock socket buffer as uiomove may sleep. */
--=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86ehzwwt6a.fsf>