Hi,

On Tue, 06 Mar 2012 20:50:34 +0100 Andre Oppermann wrote:

 AO> 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:

 AO> Hi Mikolaj,

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

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

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

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

With this patch smb mount fails with the error:

smb_iod_recvall: tran return NULL without error

 AO> Index: sys/kern/uipc_socket.c
 AO> ===================================================================
 AO> --- sys/kern/uipc_socket.c (revision 232616)
 AO> +++ sys/kern/uipc_socket.c (revision 232617)
 AO> @@ -2044,7 +2044,7 @@ deliver:
 AO>    if (mp0 != NULL) {
 AO>            /* Dequeue as many mbufs as possible. */
 AO>            if (!(flags & MSG_PEEK) && len >= sb->sb_mb->m_len) {
 AO> -                  for (*mp0 = m = sb->sb_mb;
 AO> +                  for (m = sb->sb_mb;
 AO>                         m != NULL && m->m_len <= len;
 AO>                         m = m->m_next) {
 AO>                            len -= m->m_len;
 AO> @@ -2052,10 +2052,15 @@ deliver:
 AO>                            sbfree(sb, m);
 AO>                            n = m;
 AO>                    }
 AO> +                  n->m_next = NULL;
 AO>                    sb->sb_mb = m;
 AO> +                  sb->sb_lastrecord = sb->sb_mb;
 AO>                    if (sb->sb_mb == NULL)
 AO>                            SB_EMPTY_FIXUP(sb);
 AO> -                  n->m_next = NULL;
 AO> +                  if (*mp0 != NULL)
 AO> +                          m_cat(*mp0, m);
 AO> +                  else
 AO> +                          *mp0 = m;
 AO>            }

At that moment m points to the end of the chain. Shouldn't *mp0 be set to
sb->sb_mb before the "for" loop?

 AO>            /* Copy the remainder. */
 AO>            if (len > 0) {
 AO> @@ -2066,9 +2071,9 @@ deliver:
 AO>                    if (m == NULL)
 AO>                            len = 0;        /* Don't flush data from 
sockbuf. */
 AO>                    else
 AO> -                          uio->uio_resid -= m->m_len;
 AO> +                          uio->uio_resid -= len;
 AO>                    if (*mp0 != NULL)
 AO> -                          n->m_next = m;
 AO> +                          m_cat(*mp0, m);
 AO>                    else
 AO>                            *mp0 = m;
 AO>                    if (*mp0 == NULL) {
 AO> 

-- 
Mikolaj Golub
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to