On 08.03.2012 11:53, Mikolaj Golub wrote:
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?

Yes, doesn't compute this way.  I've put in your fix in this revision:

http://svn.freebsd.org/changeset/base/232867

--
Andre

  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>


_______________________________________________
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