Re: [PATCH][UNIX] EOF on non-blocking SOCK_SEQPACKET

[Herbert Xu]

On Tue, Nov 27, 2007 at 09:33:23AM +, Florian Zumbiehl wrote:
> 
> I am not absolutely sure whether this actually is a bug (as in: I've got
> no clue what the standards say or what other implementations do), but at
> least I was pretty surprised when I noticed that a recv() on a
> non-blocking unix domain socket of type SOCK_SEQPACKET (which is connection
> oriented, after all) where the remote end has closed the connection
> returned -1 (EAGAIN) rather than 0 to indicate end of file.

I agree with your expectation.  In fact, that's what POSIX says too.
Since the risk of this breaking an existing application seems to be
minimal, I've applied your patch to net-2.6.

However, I had to reformat it so that it fits with the Linux coding
style.  Please take this into account for future patches.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[EMAIL PROTECTED]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index e835da8..060bba4 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1637,8 +1637,15 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct 
socket *sock,
mutex_lock(&u->readlock);
 
skb = skb_recv_datagram(sk, flags, noblock, &err);
-   if (!skb)
+   if (!skb) {
+   unix_state_lock(sk);
+   /* Signal EOF on disconnected non-blocking SEQPACKET socket. */
+   if (sk->sk_type == SOCK_SEQPACKET && err == -EAGAIN &&
+   (sk->sk_shutdown & RCV_SHUTDOWN))
+   err = 0;
+   unix_state_unlock(sk);
goto out_unlock;
+   }
 
wake_up_interruptible_sync(&u->peer_wait);
 
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][UNIX] EOF on non-blocking SOCK_SEQPACKET

[Florian Zumbiehl]

Hi,

I am not absolutely sure whether this actually is a bug (as in: I've got
no clue what the standards say or what other implementations do), but at
least I was pretty surprised when I noticed that a recv() on a
non-blocking unix domain socket of type SOCK_SEQPACKET (which is connection
oriented, after all) where the remote end has closed the connection
returned -1 (EAGAIN) rather than 0 to indicate end of file.

This is a test case:

| #include 
| #include 
| #include 
| #include 
| #include 
| #include 
| #include 
| 
| int main(){
|   int sock;
|   struct sockaddr_un addr;
|   char buf[4096];
|   int pfds[2];
| 
|   pipe(pfds);
|   sock=socket(PF_UNIX,SOCK_SEQPACKET,0);
|   addr.sun_family=AF_UNIX;
|   strcpy(addr.sun_path,"/tmp/foobar_testsock");
|   bind(sock,(struct sockaddr *)&addr,sizeof(addr));
|   listen(sock,1);
|   if(fork()){
|   close(sock);
|   sock=socket(PF_UNIX,SOCK_SEQPACKET,0);
|   connect(sock,(struct sockaddr *)&addr,sizeof(addr));
|   fcntl(sock,F_SETFL,fcntl(sock,F_GETFL)|O_NONBLOCK);
|   close(pfds[1]);
|   read(pfds[0],buf,sizeof(buf));
|   recv(sock,buf,sizeof(buf),0); // <-- this one
|   }else accept(sock,NULL,NULL);
|   exit(0);
| }

If you try it, make sure /tmp/foobar_testsock doesn't exist.

The marked recv() returns -1 (EAGAIN) on 2.6.23.9. Below you find a
patch that fixes that.

Florian

---
Signed-off-by: Florian Zumbiehl <[EMAIL PROTECTED]>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index a05c342..fa0aec5 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1632,8 +1632,13 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct 
socket *sock,
mutex_lock(&u->readlock);
 
skb = skb_recv_datagram(sk, flags, noblock, &err);
-   if (!skb)
+   if (!skb) {
+   unix_state_lock(sk);
+   if (sk->sk_type==SOCK_SEQPACKET && err==-EAGAIN && 
(sk->sk_shutdown&RCV_SHUTDOWN))
+   err=0; /* signal EOF on disconnected non-blocking 
SEQPACKET socket */
+   unix_state_unlock(sk);
goto out_unlock;
+   }
 
wake_up_interruptible(&u->peer_wait);
 
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html