All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jiri Bohac <jbohac@suse.cz>
To: David Miller <davem@davemloft.net>
Cc: jbohac@suse.cz, arnd@arndb.de, acme@ghostprotocols.net,
	netdev@vger.kernel.org
Subject: [PATCH v4] ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg
Date: Wed, 19 Nov 2014 23:05:49 +0100	[thread overview]
Message-ID: <20141119220549.GA26133@midget.suse.cz> (raw)
In-Reply-To: <20141119.154400.1045032776950540216.davem@davemloft.net>

This fixes an old regression introduced by commit
b0d0d915 (ipx: remove the BKL).

When a recvmsg syscall blocks waiting for new data, no data can be sent on the
same socket with sendmsg because ipx_recvmsg() sleeps with the socket locked.

This breaks mars-nwe (NetWare emulator):
- the ncpserv process reads the request using recvmsg
- ncpserv forks and spawns nwconn
- ncpserv calls a (blocking) recvmsg and waits for new requests
- nwconn deadlocks in sendmsg on the same socket 

Commit b0d0d915 has simply replaced BKL locking with
lock_sock/release_sock. Unlike now, BKL got unlocked while
sleeping, so a blocking recvmsg did not block a concurrent
sendmsg.

Only keep the socket locked while actually working with the socket data and
release it prior to calling skb_recv_datagram(). 


Signed-off-by: Jiri Bohac <jbohac@suse.cz>

diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index a0c7536..d0725d9 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1764,6 +1764,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
 	struct ipxhdr *ipx = NULL;
 	struct sk_buff *skb;
 	int copied, rc;
+	bool locked = true;
 
 	lock_sock(sk);
 	/* put the autobinding in */
@@ -1790,6 +1791,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
 	if (sock_flag(sk, SOCK_ZAPPED))
 		goto out;
 
+	release_sock(sk);
+	locked = false;
 	skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
 				flags & MSG_DONTWAIT, &rc);
 	if (!skb) {
@@ -1825,7 +1828,8 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
 out_free:
 	skb_free_datagram(sk, skb);
 out:
-	release_sock(sk);
+	if (locked)
+		release_sock(sk);
 	return rc;
 }
 
-- 
Jiri Bohac <jbohac@suse.cz>
SUSE Labs, SUSE CZ

  reply	other threads:[~2014-11-19 22:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-17  1:34 ipx: fix locking regression in ipx_sendmsg and ipx_recvmsg Jiri Bohac
2014-11-18 13:37 ` Arnd Bergmann
2014-11-18 20:49   ` David Miller
2014-11-18 22:10   ` Jiri Bohac
2014-11-18 22:24     ` [PATCH v2] " Jiri Bohac
2014-11-19  8:32     ` Arnd Bergmann
2014-11-19 10:34       ` Jiri Bohac
2014-11-19 10:38         ` [PATCH v3] " Jiri Bohac
2014-11-19 10:50           ` Arnd Bergmann
2014-11-19 14:38           ` Sergei Shtylyov
2014-11-19 20:44           ` David Miller
2014-11-19 22:05             ` Jiri Bohac [this message]
2014-11-19 22:12               ` [PATCH v4] ipx: " Arnd Bergmann
2014-11-21 19:46               ` David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141119220549.GA26133@midget.suse.cz \
    --to=jbohac@suse.cz \
    --cc=acme@ghostprotocols.net \
    --cc=arnd@arndb.de \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.