Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 05 Sep 2011 22:58:49 +0300
From:      Mikolaj Golub <trociny@freebsd.org>
To:        Andre Oppermann <andre@FreeBSD.org>
Cc:        Kostik Belousov <kib@FreeBSD.org>, freebsd-net@freebsd.org, Pawel Jakub Dawidek <pjd@FreeBSD.org>
Subject:   Re: soreceive_stream: mbuf leak if called with mp0 and MSG_WAITALL
Message-ID:  <86r53uhibq.fsf@kopusha.home.net>
In-Reply-To: <86ehzwwt6a.fsf@kopusha.home.net> (Mikolaj Golub's message of "Sun, 04 Sep 2011 12:30:53 %2B0300")
References:  <86ehzwwt6a.fsf@kopusha.home.net>

next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=


On Sun, 04 Sep 2011 12:30:53 +0300 Mikolaj Golub wrote:

 MG> Apparently soreceive_stream() has an issue if it is called to receive data as a
 MG> mbuf chain (by supplying an non zero mbuf **mp0) and with MSG_WAITALL set.

 MG> I ran into this issue with smbfs, which uses soreceive() exactly in this way
 MG> (see netsmb/smb_trantcp.c:nbssn_recv()).

Stressing smbfs a little I also observed the following soreceive_stream()
related panic:

#9  0x80a28c80 in panic (fmt=0x80f4b4a4 "sbappendstream 1")
    at /usr/src/sys/kern/kern_shutdown.c:606
#10 0x80a9746b in sbappendstream_locked (sb=0x8bff1874, m=0x8885a600)
    at /usr/src/sys/kern/uipc_sockbuf.c:527
#11 0x80bcef62 in tcp_do_segment (m=0x8885a600, th=0x8885a674, so=0x8bff1820, tp=0x8bb4f560, 
    drop_hdrlen=52, tlen=51, iptos=0 '\0', ti_locked=1)
    at /usr/src/sys/netinet/tcp_input.c:2854
#12 0x80bd091d in tcp_input (m=0x8885a600, off0=20) at /usr/src/sys/netinet/tcp_input.c:1382
#13 0x80b5b4fe in ip_input (m=0x8885a600) at /usr/src/sys/netinet/ip_input.c:765
#14 0x80af504b in swi_net (arg=0x81825880) at /usr/src/sys/net/netisr.c:806
#15 0x809fd535 in intr_event_execute_handlers (p=0x86ddc588, ie=0x86d37200)
    at /usr/src/sys/kern/kern_intr.c:1257
#16 0x809fe419 in ithread_loop (arg=0x86d39bb0) at /usr/src/sys/kern/kern_intr.c:1270
#17 0x809fa7a8 in fork_exit (callout=0x809fe370 <ithread_loop>, arg=0x86d39bb0, 
    frame=0x86926d28) at /usr/src/sys/kern/kern_fork.c:1029
#18 0x80d68914 in fork_trampoline () at /usr/src/sys/i386/i386/exception.s:275
(kgdb) fr 10
#10 0x80a9746b in sbappendstream_locked (sb=0x8bff1874, m=0x8885a600)
    at /usr/src/sys/kern/uipc_sockbuf.c:527
527             KASSERT(sb->sb_mb == sb->sb_lastrecord,("sbappendstream 1"));
(kgdb) l
522     sbappendstream_locked(struct sockbuf *sb, struct mbuf *m)
523     {
524             SOCKBUF_LOCK_ASSERT(sb);
525     
526             KASSERT(m->m_nextpkt == NULL,("sbappendstream 0"));
527             KASSERT(sb->sb_mb == sb->sb_lastrecord,("sbappendstream 1"));
528     
529             SBLASTMBUFCHK(sb);
530     
531             sbcompress(sb, m, sb->sb_mbtail);
(kgdb) p m
$1 = (struct mbuf *) 0x8885a600
(kgdb) p m->m_hdr.mh_next
$2 = (struct mbuf *) 0x0
(kgdb) p sb->sb_mb
$3 = (struct mbuf *) 0x93965e00
(kgdb) p sb->sb_lastrecord
$4 = (struct mbuf *) 0x88cb0200
(kgdb) p sb
$5 = (struct sockbuf *) 0x8bff1874

This sb belonged to smb_iod_thread which at that time was in
soreceive_stream(), notifying the protocol that buffer had been drained:

#1  0x80d74cb7 in ipi_nmi_handler () at /usr/src/sys/i386/i386/mp_machdep.c:1478
#2  0x80d7f383 in trap (frame=0xdc33ea58) at /usr/src/sys/i386/i386/trap.c:219
#3  0x80d6886c in calltrap () at /usr/src/sys/i386/i386/exception.s:168
#4  0x80a26955 in _rw_wlock_hard (rw=0x8d18fac0, tid=2285360576, 
    file=0x80f68ceb "/usr/src/sys/netinet/tcp_usrreq.c", line=732) at cpufunc.h:294
#5  0x80a274d6 in _rw_wlock (rw=0x8d18fac0, 
    file=0x80f68ceb "/usr/src/sys/netinet/tcp_usrreq.c", line=732)
    at /usr/src/sys/kern/kern_rwlock.c:240
#6  0x80bdf585 in tcp_usr_rcvd (so=0x8bff1820, flags=64)
    at /usr/src/sys/netinet/tcp_usrreq.c:732
#7  0x80a9cf63 in soreceive_stream (so=0x8bff1820, psa=0x0, uio=0xdc33ec10, mp0=0xdc33ec44, 
    controlp=0x0, flagsp=0xdc33ec40) at /usr/src/sys/kern/uipc_socket.c:2097
#8  0x80a9a6c9 in soreceive (so=0x8bff1820, psa=0x0, uio=0xdc33ec10, mp0=0xdc33ec44, 
    controlp=0x0, flagsp=0xdc33ec40) at /usr/src/sys/kern/uipc_socket.c:2309
#9  0x91165e14 in nbssn_recv (nbp=0x874a49c0, mpp=0xdc33ec98, lenp=0xdc33ec64, 
    rpcodep=0xdc33ec6b "", td=0x8837d5c0)
    at /usr/src/sys/modules/smbfs/../../netsmb/smb_trantcp.c:378
#10 0x91165fee in smb_nbst_recv (vcp=0x8961ae00, mpp=0xdc33ec98, td=0x8837d5c0)
    at /usr/src/sys/modules/smbfs/../../netsmb/smb_trantcp.c:598
#11 0x9116bda1 in smb_iod_recvall (iod=0x88c64980)
    at /usr/src/sys/modules/smbfs/../../netsmb/smb_iod.c:305
#12 0x9116c82c in smb_iod_thread (arg=0x88c64980)
    at /usr/src/sys/modules/smbfs/../../netsmb/smb_iod.c:645
#13 0x809fa7a8 in fork_exit (callout=0x9116c600 <smb_iod_thread>, arg=0x88c64980, 
    frame=0xdc33ed28) at /usr/src/sys/kern/kern_fork.c:1029
#14 0x80d68914 in fork_trampoline () at /usr/src/sys/i386/i386/exception.s:275
(kgdb) fr 7
#7  0x80a9cf63 in soreceive_stream (so=0x8bff1820, psa=0x0, uio=0xdc33ec10, mp0=0xdc33ec44, 
    controlp=0x0, flagsp=0xdc33ec40) at /usr/src/sys/kern/uipc_socket.c:2097
2097                            (*so->so_proto->pr_usrreqs->pru_rcvd)(so, flags);
(kgdb) l
2092                    if ((so->so_proto->pr_flags & PR_WANTRCVD) &&
2093                        (((flags & MSG_WAITALL) && uio->uio_resid > 0) ||
2094                         !(flags & MSG_SOCALLBCK))) {
2095                            SOCKBUF_UNLOCK(sb);
2096                            VNET_SO_ASSERT(so);
2097                            (*so->so_proto->pr_usrreqs->pru_rcvd)(so, flags);
2098                            SOCKBUF_LOCK(sb);
2099                    }
2100            }
2101    
(kgdb) p sb
$6 = (struct sockbuf *) 0x8bff1874
(kgdb) p uio->uio_resid
$7 = 0

I think that after draining mbufs in the "Dequeue as many mbufs as possible"
part, and if there is still data in the socket buffer, we have to update
sb_lastrecord (as it is done in soreceive_generic). See the attached patch.

-- 
Mikolaj Golub


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=uipc_socket.c.soreceive_stream.2.patch

Index: sys/kern/uipc_socket.c
===================================================================
--- sys/kern/uipc_socket.c	(revision 225405)
+++ sys/kern/uipc_socket.c	(working copy)
@@ -2041,6 +2041,8 @@
 			sb->sb_mb = m;
 			if (sb->sb_mb == NULL)
 				SB_EMPTY_FIXUP(sb);
+			else if (sb->sb_mb->m_nextpkt == NULL)
+				sb->sb_lastrecord = sb->sb_mb;
 			n->m_next = NULL;
 		}
 		/* Copy the remainder. */

--=-=-=--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86r53uhibq.fsf>