diff mbox series

socket: fix blocking udp recvfrom.

Message ID CACxphuFGAODB_=X1fVFqJa=U14E_YSJMreWJcOgzdsooke1wxg@mail.gmail.com
State New
Headers show
Series socket: fix blocking udp recvfrom. | expand

Commit Message

Vic Lee Feb. 28, 2019, 11:59 a.m. UTC 
Sometimes sorecvfrom() is called from slirp.c because revents == G_IO_IN,
however inside sorecvfrom() function, ioctlsocket() returns 0 bytes available
and recvfrom could be blocking indefinitely. This adds a non-blocking flag to
recvfrom and checks data availability.
---
 slirp/socket.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Samuel Thibault Feb. 28, 2019, 8:51 p.m. UTC | #1 
Hello,

llyzs, le jeu. 28 févr. 2019 19:59:12 +0800, a ecrit:
> Sometimes sorecvfrom() is called from slirp.c because revents == G_IO_IN,
> however inside sorecvfrom() function, ioctlsocket() returns 0 bytes available
> and recvfrom could be blocking indefinitely. This adds a non-blocking flag to
> recvfrom and checks data availability.

When ioctlsocket() returns 0 bytes available, we could as well just
immediately return, without even calling recvfrom. We could just move
that ioctlsocket call above the m_get() call, so we can just return
without having to clean up anything.

This however still looks weird. AFAIK, G_IO_IN means that we can make
one recv call and be sure that it won't block. Do you have an idea why
it wouldn't necessarily be the case?

Samuel
Vic Lee March 1, 2019, 4:27 a.m. UTC | #2 
Hi,

I believe this is because UDP packet can have 0 length payload. I
quote some resources here:
https://stackoverflow.com/questions/12505892/under-linux-can-recv-ever-return-0-on-udp
https://stackoverflow.com/questions/5307031/how-to-detect-receipt-of-a-0-length-udp-datagram

I also found out that sometimes ioctlsocket() call can return error
(probably caused by my unstable wifi linux driver), in which case we
can also return immediately.

I will submit a new version of the patch according to your suggestions.

On Fri, 1 Mar 2019 at 04:51, Samuel Thibault <samuel.thibault@gnu.org> wrote:
>
> Hello,
>
> llyzs, le jeu. 28 févr. 2019 19:59:12 +0800, a ecrit:
> > Sometimes sorecvfrom() is called from slirp.c because revents == G_IO_IN,
> > however inside sorecvfrom() function, ioctlsocket() returns 0 bytes available
> > and recvfrom could be blocking indefinitely. This adds a non-blocking flag to
> > recvfrom and checks data availability.
>
> When ioctlsocket() returns 0 bytes available, we could as well just
> immediately return, without even calling recvfrom. We could just move
> that ioctlsocket call above the m_get() call, so we can just return
> without having to clean up anything.
>
> This however still looks weird. AFAIK, G_IO_IN means that we can make
> one recv call and be sure that it won't block. Do you have an idea why
> it wouldn't necessarily be the case?
>
> Samuel
Samuel Thibault March 1, 2019, 5:12 a.m. UTC | #3 
llyzs, le ven. 01 mars 2019 12:27:31 +0800, a ecrit:
> I believe this is because UDP packet can have 0 length payload.

Ah, right, that one makes sense indeed.

> I also found out that sometimes ioctlsocket() call can return error
> (probably caused by my unstable wifi linux driver), in which case we
> can also return immediately.

We should check for the value returned by ioctlsocket() then, otherwise
the value of n is undefined.

Samuel
diff mbox series

Patch

diff --git a/slirp/socket.c b/slirp/socket.c
index c01d8696af..ea30478ce6 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -581,7 +581,7 @@  sorecvfrom(struct socket *so)
          }
          /* } */

-         m->m_len = recvfrom(so->s, m->m_data, len, 0,
+         m->m_len = recvfrom(so->s, m->m_data, len, MSG_DONTWAIT,
                              (struct sockaddr *)&addr, &addrlen);
          DEBUG_MISC((dfd, " did recvfrom %d, errno = %d-%s\n",
                      m->m_len, errno,strerror(errno)));
@@ -618,6 +618,8 @@  sorecvfrom(struct socket *so)
              break;
            }
            m_free(m);
+         } else if (m->m_len==0) {
+           m_free(m);
          } else {
          /*
           * Hack: domain name lookup will be used the most for UDP,