soreceive_stream: mbuf leak if called with mp0 and MSG_WAITALL

[Andre Oppermann]

On 05.09.2011 21:58, Mikolaj Golub wrote:
>
> 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:

Hi Mikolaj,

thank you very much for testing, reporting and fixing bugs in soreceive_stream().

I've altered your proposed patches a bit and committed them into my workqueue
with the following revisions:

http://svn.freebsd.org/changeset/base/232617
http://svn.freebsd.org/changeset/base/232618

Would you mind testing them again before they go into HEAD?

-- 
Andre

> #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.
>