Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1243026 > unrolled thread

[PATCH v4 0/3] net: unix: fix use-after-free

Back to article view | Back to linux.kernel

Contents

  [PATCH v4 0/3] net: unix: fix use-after-free Jason Baron <jbaron@akamai.com> - 2015-10-09 06:20 +0200
    [PATCH v4 3/3] net: unix: optimize wakeups in unix_dgram_recvmsg() Jason Baron <jbaron@akamai.com> - 2015-10-09 06:20 +0200
      Re: [PATCH v4 3/3] net: unix: optimize wakeups in  unix_dgram_recvmsg() kbuild test robot <lkp@intel.com> - 2015-10-09 06:40 +0200
        Re: [PATCH v4 3/3] net: unix: optimize wakeups in unix_dgram_recvmsg() Jason Baron <jbaron@akamai.com> - 2015-10-09 17:20 +0200
    [PATCH v4 2/3] net: unix: Convert gc_flags to flags Jason Baron <jbaron@akamai.com> - 2015-10-09 06:20 +0200
    Re: [PATCH v4 0/3] net: unix: fix use-after-free David Miller <davem@davemloft.net> - 2015-10-11 13:50 +0200
      Re: [PATCH v4 0/3] net: unix: fix use-after-free Rainer Weikusat <rweikusat@mobileactivedefense.com> - 2015-10-12 15:00 +0200
        Re: [PATCH v4 0/3] net: unix: fix use-after-free Eric Dumazet <eric.dumazet@gmail.com> - 2015-10-12 15:40 +0200
      Re: [PATCH v4 0/3] net: unix: fix use-after-free Jason Baron <jbaron@akamai.com> - 2015-10-12 22:00 +0200
        Re: [PATCH v4 0/3] net: unix: fix use-after-free David Miller <davem@davemloft.net> - 2015-10-13 03:40 +0200

#1243026 — [PATCH v4 0/3] net: unix: fix use-after-free

Hi,

These patches are against mainline, I can re-base to net-next, please
let me know.

They have been tested against: https://lkml.org/lkml/2015/9/13/195,
which causes the use-after-free quite quickly and here:
https://lkml.org/lkml/2015/10/2/693.

Thanks,

-Jason

v4:
-set UNIX_NOSPACE only if the peer socket has receive space

v3:
-beef up memory barrier comments in 3/3 (Peter Zijlstra)
-clean up unix_dgram_writable() function in 3/3 (Joe Perches)

Jason Baron (3):
  net: unix: fix use-after-free in unix_dgram_poll()
  net: unix: Convert gc_flags to flags
  net: unix: optimize wakeups in unix_dgram_recvmsg()

 include/net/af_unix.h |   4 +-
 net/unix/af_unix.c    | 124 ++++++++++++++++++++++++++++++++++++++++----------
 net/unix/garbage.c    |  12 ++---
 3 files changed, 108 insertions(+), 32 deletions(-)

-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1243027 — [PATCH v4 3/3] net: unix: optimize wakeups in unix_dgram_recvmsg()

Now that connect() permanently registers a callback routine, we can induce
extra overhead in unix_dgram_recvmsg(), which unconditionally wakes up
its peer_wait queue on every receive. This patch makes the wakeup there
conditional on there being waiters.

Tested using: http://www.spinics.net/lists/netdev/msg145533.html

Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 include/net/af_unix.h |  1 +
 net/unix/af_unix.c    | 92 +++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 69 insertions(+), 24 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 6a4a345..cf21ffd 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -61,6 +61,7 @@ struct unix_sock {
 	unsigned long		flags;
 #define UNIX_GC_CANDIDATE	0
 #define UNIX_GC_MAYBE_CYCLE	1
+#define UNIX_NOSPACE		2
 	struct socket_wq	peer_wq;
 	wait_queue_t		wait;
 };
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f789423..05fbd00 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -326,7 +326,7 @@ found:
 	return s;
 }
 
-static inline int unix_writable(struct sock *sk)
+static inline bool unix_writable(struct sock *sk)
 {
 	return (atomic_read(&sk->sk_wmem_alloc) << 2) <= sk->sk_sndbuf;
 }
@@ -1079,6 +1079,12 @@ static long unix_wait_for_peer(struct sock *other, long timeo)
 
 	prepare_to_wait_exclusive(&u->peer_wait, &wait, TASK_INTERRUPTIBLE);
 
+	set_bit(UNIX_NOSPACE, &u->flags);
+	/* Ensure that we either see space in the peer sk_receive_queue via the
+	 * unix_recvq_full() check below, or we receive a wakeup when it
+	 * empties. Pairs with the mb in unix_dgram_recvmsg().
+	 */
+	smp_mb__after_atomic();
 	sched = !sock_flag(other, SOCK_DEAD) &&
 		!(other->sk_shutdown & RCV_SHUTDOWN) &&
 		unix_recvq_full(other);
@@ -1623,17 +1629,27 @@ restart:
 
 	if (unix_peer(other) != sk && unix_recvq_full(other)) {
 		if (!timeo) {
-			err = -EAGAIN;
-			goto out_unlock;
-		}
-
-		timeo = unix_wait_for_peer(other, timeo);
+			set_bit(UNIX_NOSPACE, &unix_sk(other)->flags);
+			/* Ensure that we either see space in the peer
+			 * sk_receive_queue via the unix_recvq_full() check
+			 * below, or we receive a wakeup when it empties. This
+			 * makes sure that epoll ET triggers correctly. Pairs
+			 * with the mb in unix_dgram_recvmsg().
+			 */
+			smp_mb__after_atomic();
+			if (unix_recvq_full(other)) {
+				err = -EAGAIN;
+				goto out_unlock;
+			}
+		} else {
+			timeo = unix_wait_for_peer(other, timeo);
 
-		err = sock_intr_errno(timeo);
-		if (signal_pending(current))
-			goto out_free;
+			err = sock_intr_errno(timeo);
+			if (signal_pending(current))
+				goto out_free;
 
-		goto restart;
+			goto restart;
+		}
 	}
 
 	if (sock_flag(other, SOCK_RCVTSTAMP))
@@ -1939,8 +1955,19 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
 		goto out_unlock;
 	}
 
-	wake_up_interruptible_sync_poll(&u->peer_wait,
-					POLLOUT | POLLWRNORM | POLLWRBAND);
+	/* Ensure that waiters on our sk->sk_receive_queue draining that check
+	 * via unix_recvq_full() either see space in the queue or get a wakeup
+	 * below. sk->sk_receive_queue is reduece by the __skb_recv_datagram()
+	 * call above. Pairs with the mb in unix_dgram_sendmsg(),
+	 *unix_dgram_poll(), and unix_wait_for_peer().
+	 */
+	smp_mb();
+	if (test_bit(UNIX_NOSPACE, &u->flags)) {
+		clear_bit(UNIX_NOSPACE, &u->flags);
+		wake_up_interruptible_sync_poll(&u->peer_wait,
+						POLLOUT | POLLWRNORM |
+						POLLWRBAND);
+	}
 
 	if (msg->msg_name)
 		unix_copy_addr(msg, skb->sk);
@@ -2432,11 +2459,25 @@ static unsigned int unix_poll(struct file *file, struct socket *sock, poll_table
 	return mask;
 }
 
+static bool unix_dgram_writable(struct sock *sk, struct sock *other,
+				bool *other_nospace)
+{
+	*other_full = false;
+
+	if (other && unix_peer(other) != sk && unix_recvq_full(other)) {
+		*other_full = true;
+		return false;
+	}
+
+	return unix_writable(sk);
+}
+
 static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
 				    poll_table *wait)
 {
 	struct sock *sk = sock->sk, *other;
-	unsigned int mask, writable;
+	unsigned int mask;
+	bool other_nospace;
 
 	sock_poll_wait(file, sk_sleep(sk), wait);
 	mask = 0;
@@ -2468,20 +2509,23 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
 	if (!(poll_requested_events(wait) & (POLLWRBAND|POLLWRNORM|POLLOUT)))
 		return mask;
 
-	writable = unix_writable(sk);
 	other = unix_peer_get(sk);
-	if (other) {
-		if (unix_peer(other) != sk) {
-			if (unix_recvq_full(other))
-				writable = 0;
-		}
-		sock_put(other);
-	}
-
-	if (writable)
+	if (unix_dgram_writable(sk, other, &other_nospace)) {
 		mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
-	else
+	} else {
 		set_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
+		if (other_nospace)
+			set_bit(UNIX_NOSPACE, &unix_sk(other)->flags);
+		/* Ensure that we either see space in the peer sk_receive_queue
+		 * via the unix_recvq_full() check below, or we receive a wakeup
+		 * when it empties. Pairs with the mb in unix_dgram_recvmsg().
+		 */
+		smp_mb__after_atomic();
+		if (unix_dgram_writable(sk, other, &other_nospace))
+			mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
+	}
+	if (other)
+		sock_put(other);
 
 	return mask;
 }
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243035 — Re: [PATCH v4 3/3] net: unix: optimize wakeups in unix_dgram_recvmsg()

[Multipart message — attachments visible in raw view] — view raw

Hi Jason,

[auto build test ERROR on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: x86_64-randconfig-i0-201540 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   net/unix/af_unix.c: In function 'unix_dgram_writable':
>> net/unix/af_unix.c:2465:3: error: 'other_full' undeclared (first use in this function)
     *other_full = false;
      ^
   net/unix/af_unix.c:2465:3: note: each undeclared identifier is reported only once for each function it appears in

vim +/other_full +2465 net/unix/af_unix.c

  2459		return mask;
  2460	}
  2461	
  2462	static bool unix_dgram_writable(struct sock *sk, struct sock *other,
  2463					bool *other_nospace)
  2464	{
> 2465		*other_full = false;
  2466	
  2467		if (other && unix_peer(other) != sk && unix_recvq_full(other)) {
  2468			*other_full = true;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1243460 — Re: [PATCH v4 3/3] net: unix: optimize wakeups in unix_dgram_recvmsg()

On 10/09/2015 12:29 AM, kbuild test robot wrote:
> Hi Jason,
> 
> [auto build test ERROR on v4.3-rc3 -- if it's inappropriate base, please ignore]
> 
> config: x86_64-randconfig-i0-201540 (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    net/unix/af_unix.c: In function 'unix_dgram_writable':
>>> net/unix/af_unix.c:2465:3: error: 'other_full' undeclared (first use in this function)
>      *other_full = false;
>       ^
>    net/unix/af_unix.c:2465:3: note: each undeclared identifier is reported only once for each function it appears in
> 


Forgot to refresh this patch before sending. The one that I tested with
is below.

Thanks,

-Jason




Now that connect() permanently registers a callback routine, we can induce
extra overhead in unix_dgram_recvmsg(), which unconditionally wakes up
its peer_wait queue on every receive. This patch makes the wakeup there
conditional on there being waiters.

Tested using: http://www.spinics.net/lists/netdev/msg145533.html

Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 include/net/af_unix.h |  1 +
 net/unix/af_unix.c    | 92 +++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 69 insertions(+), 24 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 6a4a345..cf21ffd 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -61,6 +61,7 @@ struct unix_sock {
 	unsigned long		flags;
 #define UNIX_GC_CANDIDATE	0
 #define UNIX_GC_MAYBE_CYCLE	1
+#define UNIX_NOSPACE		2
 	struct socket_wq	peer_wq;
 	wait_queue_t		wait;
 };
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f789423..ac9bcd8 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -326,7 +326,7 @@ found:
 	return s;
 }
 
-static inline int unix_writable(struct sock *sk)
+static inline bool unix_writable(struct sock *sk)
 {
 	return (atomic_read(&sk->sk_wmem_alloc) << 2) <= sk->sk_sndbuf;
 }
@@ -1079,6 +1079,12 @@ static long unix_wait_for_peer(struct sock *other, long timeo)
 
 	prepare_to_wait_exclusive(&u->peer_wait, &wait, TASK_INTERRUPTIBLE);
 
+	set_bit(UNIX_NOSPACE, &u->flags);
+	/* Ensure that we either see space in the peer sk_receive_queue via the
+	 * unix_recvq_full() check below, or we receive a wakeup when it
+	 * empties. Pairs with the mb in unix_dgram_recvmsg().
+	 */
+	smp_mb__after_atomic();
 	sched = !sock_flag(other, SOCK_DEAD) &&
 		!(other->sk_shutdown & RCV_SHUTDOWN) &&
 		unix_recvq_full(other);
@@ -1623,17 +1629,27 @@ restart:
 
 	if (unix_peer(other) != sk && unix_recvq_full(other)) {
 		if (!timeo) {
-			err = -EAGAIN;
-			goto out_unlock;
-		}
-
-		timeo = unix_wait_for_peer(other, timeo);
+			set_bit(UNIX_NOSPACE, &unix_sk(other)->flags);
+			/* Ensure that we either see space in the peer
+			 * sk_receive_queue via the unix_recvq_full() check
+			 * below, or we receive a wakeup when it empties. This
+			 * makes sure that epoll ET triggers correctly. Pairs
+			 * with the mb in unix_dgram_recvmsg().
+			 */
+			smp_mb__after_atomic();
+			if (unix_recvq_full(other)) {
+				err = -EAGAIN;
+				goto out_unlock;
+			}
+		} else {
+			timeo = unix_wait_for_peer(other, timeo);
 
-		err = sock_intr_errno(timeo);
-		if (signal_pending(current))
-			goto out_free;
+			err = sock_intr_errno(timeo);
+			if (signal_pending(current))
+				goto out_free;
 
-		goto restart;
+			goto restart;
+		}
 	}
 
 	if (sock_flag(other, SOCK_RCVTSTAMP))
@@ -1939,8 +1955,19 @@ static int unix_dgram_recvmsg(struct socket *sock, struct msghdr *msg,
 		goto out_unlock;
 	}
 
-	wake_up_interruptible_sync_poll(&u->peer_wait,
-					POLLOUT | POLLWRNORM | POLLWRBAND);
+	/* Ensure that waiters on our sk->sk_receive_queue draining that check
+	 * via unix_recvq_full() either see space in the queue or get a wakeup
+	 * below. sk->sk_receive_queue is reduece by the __skb_recv_datagram()
+	 * call above. Pairs with the mb in unix_dgram_sendmsg(),
+	 *unix_dgram_poll(), and unix_wait_for_peer().
+	 */
+	smp_mb();
+	if (test_bit(UNIX_NOSPACE, &u->flags)) {
+		clear_bit(UNIX_NOSPACE, &u->flags);
+		wake_up_interruptible_sync_poll(&u->peer_wait,
+						POLLOUT | POLLWRNORM |
+						POLLWRBAND);
+	}
 
 	if (msg->msg_name)
 		unix_copy_addr(msg, skb->sk);
@@ -2432,11 +2459,25 @@ static unsigned int unix_poll(struct file *file, struct socket *sock, poll_table
 	return mask;
 }
 
+static bool unix_dgram_writable(struct sock *sk, struct sock *other,
+				bool *other_nospace)
+{
+	*other_nospace = false;
+
+	if (other && unix_peer(other) != sk && unix_recvq_full(other)) {
+		*other_nospace = true;
+		return false;
+	}
+
+	return unix_writable(sk);
+}
+
 static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
 				    poll_table *wait)
 {
 	struct sock *sk = sock->sk, *other;
-	unsigned int mask, writable;
+	unsigned int mask;
+	bool other_nospace;
 
 	sock_poll_wait(file, sk_sleep(sk), wait);
 	mask = 0;
@@ -2468,20 +2509,23 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
 	if (!(poll_requested_events(wait) & (POLLWRBAND|POLLWRNORM|POLLOUT)))
 		return mask;
 
-	writable = unix_writable(sk);
 	other = unix_peer_get(sk);
-	if (other) {
-		if (unix_peer(other) != sk) {
-			if (unix_recvq_full(other))
-				writable = 0;
-		}
-		sock_put(other);
-	}
-
-	if (writable)
+	if (unix_dgram_writable(sk, other, &other_nospace)) {
 		mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
-	else
+	} else {
 		set_bit(SOCK_ASYNC_NOSPACE, &sk->sk_socket->flags);
+		if (other_nospace)
+			set_bit(UNIX_NOSPACE, &unix_sk(other)->flags);
+		/* Ensure that we either see space in the peer sk_receive_queue
+		 * via the unix_recvq_full() check below, or we receive a wakeup
+		 * when it empties. Pairs with the mb in unix_dgram_recvmsg().
+		 */
+		smp_mb__after_atomic();
+		if (unix_dgram_writable(sk, other, &other_nospace))
+			mask |= POLLOUT | POLLWRNORM | POLLWRBAND;
+	}
+	if (other)
+		sock_put(other);
 
 	return mask;
 }
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243028 — [PATCH v4 2/3] net: unix: Convert gc_flags to flags

Convert gc_flags to flags in perparation for the subsequent patch, which will
make use of a flag bit for a non-gc purpose.

Signed-off-by: Jason Baron <jbaron@akamai.com>
---
 include/net/af_unix.h |  2 +-
 net/unix/garbage.c    | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index 9698aff..6a4a345 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -58,7 +58,7 @@ struct unix_sock {
 	atomic_long_t		inflight;
 	spinlock_t		lock;
 	unsigned char		recursion_level;
-	unsigned long		gc_flags;
+	unsigned long		flags;
 #define UNIX_GC_CANDIDATE	0
 #define UNIX_GC_MAYBE_CYCLE	1
 	struct socket_wq	peer_wq;
diff --git a/net/unix/garbage.c b/net/unix/garbage.c
index a73a226..39794d9 100644
--- a/net/unix/garbage.c
+++ b/net/unix/garbage.c
@@ -179,7 +179,7 @@ static void scan_inflight(struct sock *x, void (*func)(struct unix_sock *),
 					 * have been added to the queues after
 					 * starting the garbage collection
 					 */
-					if (test_bit(UNIX_GC_CANDIDATE, &u->gc_flags)) {
+					if (test_bit(UNIX_GC_CANDIDATE, &u->flags)) {
 						hit = true;
 
 						func(u);
@@ -246,7 +246,7 @@ static void inc_inflight_move_tail(struct unix_sock *u)
 	 * of the list, so that it's checked even if it was already
 	 * passed over
 	 */
-	if (test_bit(UNIX_GC_MAYBE_CYCLE, &u->gc_flags))
+	if (test_bit(UNIX_GC_MAYBE_CYCLE, &u->flags))
 		list_move_tail(&u->link, &gc_candidates);
 }
 
@@ -305,8 +305,8 @@ void unix_gc(void)
 		BUG_ON(total_refs < inflight_refs);
 		if (total_refs == inflight_refs) {
 			list_move_tail(&u->link, &gc_candidates);
-			__set_bit(UNIX_GC_CANDIDATE, &u->gc_flags);
-			__set_bit(UNIX_GC_MAYBE_CYCLE, &u->gc_flags);
+			__set_bit(UNIX_GC_CANDIDATE, &u->flags);
+			__set_bit(UNIX_GC_MAYBE_CYCLE, &u->flags);
 		}
 	}
 
@@ -332,7 +332,7 @@ void unix_gc(void)
 
 		if (atomic_long_read(&u->inflight) > 0) {
 			list_move_tail(&u->link, &not_cycle_list);
-			__clear_bit(UNIX_GC_MAYBE_CYCLE, &u->gc_flags);
+			__clear_bit(UNIX_GC_MAYBE_CYCLE, &u->flags);
 			scan_children(&u->sk, inc_inflight_move_tail, NULL);
 		}
 	}
@@ -343,7 +343,7 @@ void unix_gc(void)
 	 */
 	while (!list_empty(&not_cycle_list)) {
 		u = list_entry(not_cycle_list.next, struct unix_sock, link);
-		__clear_bit(UNIX_GC_CANDIDATE, &u->gc_flags);
+		__clear_bit(UNIX_GC_CANDIDATE, &u->flags);
 		list_move_tail(&u->link, &gc_inflight_list);
 	}
 
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244106

RnJvbTogSmFzb24gQmFyb24gPGpiYXJvbkBha2FtYWkuY29tPg0KRGF0ZTogRnJpLCAgOSBPY3Qg
MjAxNSAwMDoxNTo1OSAtMDQwMA0KDQo+IFRoZXNlIHBhdGNoZXMgYXJlIGFnYWluc3QgbWFpbmxp
bmUsIEkgY2FuIHJlLWJhc2UgdG8gbmV0LW5leHQsIHBsZWFzZQ0KPiBsZXQgbWUga25vdy4NCj4g
DQo+IFRoZXkgaGF2ZSBiZWVuIHRlc3RlZCBhZ2FpbnN0OiBodHRwczovL2xrbWwub3JnL2xrbWwv
MjAxNS85LzEzLzE5NSwNCj4gd2hpY2ggY2F1c2VzIHRoZSB1c2UtYWZ0ZXItZnJlZSBxdWl0ZSBx
dWlja2x5IGFuZCBoZXJlOg0KPiBodHRwczovL2xrbWwub3JnL2xrbWwvMjAxNS8xMC8yLzY5My4N
Cg0KSSdkIGxpa2UgdG8gdW5kZXJzdGFuZCBob3cgcGF0Y2hlcyB0aGF0IGRvbid0IGV2ZW4gY29t
cGlsZSBjYW4gYmUNCiJ0ZXN0ZWQiPw0KDQpuZXQvdW5peC9hZl91bml4LmM6IEluIGZ1bmN0aW9u
IKF1bml4X2RncmFtX3dyaXRhYmxlojoNCm5ldC91bml4L2FmX3VuaXguYzoyNDgwOjM6IGVycm9y
OiChb3RoZXJfZnVsbKIgdW5kZWNsYXJlZCAoZmlyc3QgdXNlIGluIHRoaXMgZnVuY3Rpb24pDQpu
ZXQvdW5peC9hZl91bml4LmM6MjQ4MDozOiBub3RlOiBlYWNoIHVuZGVjbGFyZWQgaWRlbnRpZmll
ciBpcyByZXBvcnRlZCBvbmx5IG9uY2UgZm9yIGVhY2ggZnVuY3Rpb24gaXQgYXBwZWFycyBpbg0K
DQpDb3VsZCB5b3UgZXhwbGFpbiBob3cgdGhhdCB3b3JrcywgSSdtIGhhdmluZyBhIGhhcmQgdGlt
ZSB1bmRlcnN0YW5kaW5nDQp0aGlzPw0KDQpBbHNvIHBsZWFzZSBhZGRyZXNzIEhhbm5lcydzIGZl
ZWRiYWNrLCB0aGFua3MuDQo=
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244668

David Miller <davem@davemloft.net> writes:
> From: Jason Baron <jbaron@akamai.com>
> Date: Fri,  9 Oct 2015 00:15:59 -0400
>
>> These patches are against mainline, I can re-base to net-next, please
>> let me know.
>> 
>> They have been tested against: https://lkml.org/lkml/2015/9/13/195,
>> which causes the use-after-free quite quickly and here:
>> https://lkml.org/lkml/2015/10/2/693.
>
> I'd like to understand how patches that don't even compile can be
> "tested"?
>
> net/unix/af_unix.c: In function ‘unix_dgram_writable’:
> net/unix/af_unix.c:2480:3: error: ‘other_full’ undeclared (first use in this function)
> net/unix/af_unix.c:2480:3: note: each undeclared identifier is reported only once for each function it appears in
>
> Could you explain how that works, I'm having a hard time understanding
> this?

This is basicallly a workaround for the problem that it's not possible
to tell epoll to let go of a certain wait queue: Instead of registering
the peer_wait queue via sock_poll_wait, a wait_queue_t under control of
the af_unix.c code is linked onto it which relays a wake up on the
peer_wait queue to the 'ordinary' wait queue associated with the polled
socket via custom wake function. But (at least the code I looked it) it
enqueues a unix socket on connect which has certain side effects (in
particular, /dev/log will have a seriously large wait queue of entirely
uninterested peers) and in many cases, this is simply not necessary, as
the additional peer_wait event is only interesting in case a peer of a
fan-in socket (like /dev/log) happens to be waiting for writeabilty via
poll/ select/ epoll/ ...

Since the wait queue handling code is now under control of the af_unix.c
code, it can remove itself from the peer_wait queue prior to dropping
its reference to a peer on disconnect or on detecting a dead peer in
unix_dgram_sendmsg.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1244694

On Mon, 2015-10-12 at 13:54 +0100, Rainer Weikusat wrote:
> David Miller <davem@davemloft.net> writes:
> > From: Jason Baron <jbaron@akamai.com>
> > Date: Fri,  9 Oct 2015 00:15:59 -0400
> >
> >> These patches are against mainline, I can re-base to net-next, please
> >> let me know.
> >> 
> >> They have been tested against: https://lkml.org/lkml/2015/9/13/195,
> >> which causes the use-after-free quite quickly and here:
> >> https://lkml.org/lkml/2015/10/2/693.
> >
> > I'd like to understand how patches that don't even compile can be
> > "tested"?
> >
> > net/unix/af_unix.c: In function ‘unix_dgram_writable’:
> > net/unix/af_unix.c:2480:3: error: ‘other_full’ undeclared (first use in this function)
> > net/unix/af_unix.c:2480:3: note: each undeclared identifier is reported only once for each function it appears in
> >
> > Could you explain how that works, I'm having a hard time understanding
> > this?
> 
> This is basicallly a workaround for the problem that it's not possible
> to tell epoll to let go of a certain wait queue: Instead of registering
> the peer_wait queue via sock_poll_wait, a wait_queue_t under control of
> the af_unix.c code is linked onto it which relays a wake up on the
> peer_wait queue to the 'ordinary' wait queue associated with the polled
> socket via custom wake function. But (at least the code I looked it) it
> enqueues a unix socket on connect which has certain side effects (in
> particular, /dev/log will have a seriously large wait queue of entirely
> uninterested peers) and in many cases, this is simply not necessary, as
> the additional peer_wait event is only interesting in case a peer of a
> fan-in socket (like /dev/log) happens to be waiting for writeabilty via
> poll/ select/ epoll/ ...
> 
> Since the wait queue handling code is now under control of the af_unix.c
> code, it can remove itself from the peer_wait queue prior to dropping
> its reference to a peer on disconnect or on detecting a dead peer in
> unix_dgram_sendmsg.
> --

Okay, but David was asking how the patch was supposed to be tested, and
applied, if it does not compile.

A patch is not only showing the idea, but must be ready for inclusion.

Please ?


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245082

On 10/11/2015 07:55 AM, David Miller wrote:
> From: Jason Baron <jbaron@akamai.com>
> Date: Fri,  9 Oct 2015 00:15:59 -0400
> 
>> These patches are against mainline, I can re-base to net-next, please
>> let me know.
>>
>> They have been tested against: https://lkml.org/lkml/2015/9/13/195,
>> which causes the use-after-free quite quickly and here:
>> https://lkml.org/lkml/2015/10/2/693.
> 

Hi,

> I'd like to understand how patches that don't even compile can be
> "tested"?
> 
> net/unix/af_unix.c: In function ‘unix_dgram_writable’:
> net/unix/af_unix.c:2480:3: error: ‘other_full’ undeclared (first use in this function)
> net/unix/af_unix.c:2480:3: note: each undeclared identifier is reported only once for each function it appears in
> 
> Could you explain how that works, I'm having a hard time understanding
> this?
> 

Traveling this week, so responses a bit delayed.

Yes, I screwed up the posting. I had some outstanding code in my
local tree to make it compile, but I failed to refresh my patch series
with this outstanding code before mailing it out. So what I tested/built
was not quite what I mailed out.

As soon as I noticed this issue in patch 3/3 I re-posted it here:

http://marc.info/?l=linux-netdev&m=144440355808472&w=2

in an attempt to avoid this confusion. I'm happy to re-post the series
or whatever makes things easiest for you.

> Also please address Hannes's feedback, thanks.
> 

I've replied directly to Hannes.

Thanks,

-Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245266

From: Jason Baron <jbaron@akamai.com>
Date: Mon, 12 Oct 2015 15:50:30 -0400

> I'm happy to re-post the series or whatever makes things easiest for
> you.

This is what you should always do, reposting a single patch within a
series is not sufficient.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web