Re: possible circular locking dependency detected

Linus Torvalds writes: > On Fri, Sep 2, 2016 at 10:02 AM, Al Viro wrote: >> >> It's very much _not_ just overlayfs being pathological - that it certainly >> is, >> but the problem is much wider. > > Al, can you take a look at my two

Re: possible circular locking dependency detected

Linus Torvalds writes: > On Thu, Sep 1, 2016 at 2:43 PM, Linus Torvalds > wrote: >> On Thu, Sep 1, 2016 at 2:01 PM, Al Viro wrote: >>> >>> Outside as in "all fs activity in bind happens under it". Along with

Re: possible circular locking dependency detected

Al Viro <v...@zeniv.linux.org.uk> writes: > On Fri, Sep 02, 2016 at 04:18:04PM +0100, Rainer Weikusat wrote: > >> As far as I can tell, this should work as I can't currently imagine >> why a fs operation might end up binding a unix socket despite the >> idea to make

Re: possible circular locking dependency detected (bisected)

CAI Qian writes: > Reverted the patch below fixes this problem. > > c845acb324aa85a39650a14e7696982ceea75dc1 > af_unix: Fix splice-bind deadlock Reverting a patch fixing one deadlock in order to avoid another deadlock leaves the 'net situation' unchanged. The idea of the

[PATCH net] af_unix: Don't use continue to re-execute unix_stream_read_generic loop

are returned either way. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> Acked-by: Hannes Frederic Sowa <han...@stressinduktion.org> --- I'm resending this as the original patch seems to have been classified as superseded without anything actually superseding it. I

https://patchwork.ozlabs.org/patch/579654?

https://patchwork.ozlabs.org/patch/579654 lists this as 'superseded', among with the older versions of the patch which changed the error handling. But at least, I couldn't find anything superseding it. This was supposed to address the different-but-related problem demonstrated by the following

Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg

Philipp Hahn <pmh...@pmhahn.de> writes: > Hello Rainer, > > Am 11.02.2016 um 20:37 schrieb Rainer Weikusat: >> The unix_dgram_sendmsg routine use the following test >> >> if (unlikely(unix_peer(other) != sk && unix_recvq_full(other))) { [...] >&g

Re: [PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error

Joseph Salisbury <joseph.salisb...@canonical.com> writes: > On 02/05/2016 05:30 PM, Rainer Weikusat wrote: >> The present unix_stream_read_generic contains various code sequences of >> the form >> >> err = -EDISASTER; >> if () >> goto out;

Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg

Ben Hutchings <b...@decadent.org.uk> writes: > On Fri, 2016-02-12 at 13:25 +0000, Rainer Weikusat wrote: >> Philipp Hahn <pmh...@pmhahn.de> writes: >> > Hello Rainer, >> > >> > Am 11.02.2016 um 20:37 schrieb Rainer Weikusat: >> >

Re: [PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg

Ben Hutchings <b...@decadent.org.uk> writes: > On Fri, 2016-02-12 at 20:17 +0000, Rainer Weikusat wrote: [...] >>>> I don't think this should apply when >>>> receiving and sending sockets are identical. But that's just my >>>>

[PATCH net] af_unix: Guard against other == sk in unix_dgram_sendmsg

<pmh...@pmhahn.de> Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 29be035..f1ca279 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -1781,7 +1781,12 @@ restart_locked:

[PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error

if no data was copied because the caller didn't supply a data buffer, as demonstrated by the program available at http://pad.lv/1540731 Change it such that err is only set if an error condition was detected. Fixes: 3822b5c2fc62 Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.

Re: [PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error

code") Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- With Fixes: fixed. diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 49d5093..c1e4dd7 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2277,13 +2277,15 @@ static int unix_stream_rea

[PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error

code") Reported-by: Joseph Salisbury <joseph.salisb...@canonical.com> Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- And the subject again fixed and, since another correction was necessary, anyway, a Reported-by added. diff --git a/net/unix/af_unix.c b/net/unix/a

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: [...] > the real problem is that the function disagrees with itself on how to > use the err variable: The start uses that to record an error which > might need to be reported, the return statement uses

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: [...] > The start uses that to record an error which might need to be > reported, the return statement uses it to indicate that an error has > occurred. Hence, some kind of in-between transla

Re: [PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error

if no data was copied because the caller didn't supply a data buffer, as demonstrated by the program available at http://pad.lv/1540731 Change it such that err is only set if an error condition was detected. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- With un

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

Eric Dumazet <eric.duma...@gmail.com> writes: > On Fri, 2016-02-05 at 21:44 +0000, Rainer Weikusat wrote: >> The present unix_stream_read_generic contains various code sequences of >> the form >> >> err = -EDISASTER; >> if () >> goto out

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

Joseph Salisbury <joseph.salisb...@canonical.com> writes: > Hi Rainer, > > A kernel bug report was opened against Ubuntu [0]. After a kernel > bisect, it was found that reverting the following commit resolved this bug: > > commit 3822b5c2fc62e3de8a0f33806ff279fb7df92432 &

[PATCH] af_unix: Don't set err in unix_stream_read_generic unless there was an error

if no data was copied because the caller didn't supply a data buffer, as demonstrated by the program available at http://pad.lv/1540731 Change it such that err is only set if an error condition was detected. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- With

[PATCH] af_unix: Don't use continue to re-execute unix_stream_read_generic loop

are returned either way. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index 49d5093..3b73bd7 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2305,6 +2305,7 @@ static int unix_stream_read_generic(

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

if no data was copied because the caller didn't supply a data buffer, as demonstrated by the program available at http://pad.lv/1540731 Change it such that err is only set if an error condition was detected. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- diff --git

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > Joseph Salisbury <joseph.salisb...@canonical.com> writes: >> On 02/05/2016 02:59 PM, Rainer Weikusat wrote: > > [recvmsg w/o iovecs returning ENOTSUP for CMSG requests] [...] > There are more

Re: [V4.4-rc6 Regression] af_unix: Revert 'lock_interruptible' in stream receive code

Joseph Salisbury <joseph.salisb...@canonical.com> writes: > On 02/05/2016 02:59 PM, Rainer Weikusat wrote: [recvmsg w/o iovecs returning ENOTSUP for CMSG requests] >> Funny little problem :-). The code using the interruptible lock cleared >> err as side effect hence the >

Re: [PATCH] af_unix: Fix splice-bind deadlock

Hannes Frederic Sowa <han...@stressinduktion.org> writes: > On Sun, Jan 3, 2016, at 19:03, Rainer Weikusat wrote: [reorder i_mutex and readlock locking] > I was concerned because of the comment in skb_socket_splice: > > /* Drop the socket lock, otherwi

Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram

Eric Dumazet writes: > On Mon, 2016-01-04 at 10:10 +0100, Jacob Siverskog wrote: [...] >> I believe the crash occurred between these two actions. I just saw >> that there are some interesting events in the log prior to the crash: >> kernel: Bluetooth: Unable to push skb

Re: [PATCH] af_unix: Fix splice-bind deadlock

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > Hannes Frederic Sowa <han...@stressinduktion.org> writes: >> On 27.12.2015 21:13, Rainer Weikusat wrote: >>> -static int unix_mknod(const char *sun_path, umode_t mode, struct path *res) >>&g

Re: [PATCH] af_unix: Fix splice-bind deadlock

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: [...] > + dentry = NULL; > + if (sun_path[0]) { > + /* Get the parent directory, calculate the hash for last > + * component. > + */ > + d

Re: [PATCH] af_unix: Fix splice-bind deadlock

described above). Dmitry Vyukov(<dvyu...@google.com>) tested the original patch. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- This fixes two 'wrong' error returns, namely, return -EADDRINUSE if kern_path_create returned -EEXIST but delay returning an error from kern

Re: [PATCH] af_unix: Fix splice-bind deadlock

Hannes Frederic Sowa <han...@stressinduktion.org> writes: > On 27.12.2015 21:13, Rainer Weikusat wrote: >> -static int unix_mknod(const char *sun_path, umode_t mode, struct path *res) >> +static int unix_mknod(struct dentry *dentry, struct path *path, umode_t >> mode, &

Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram

Jacob Siverskog <jacob@teenage.engineering> writes: > On Tue, Dec 29, 2015 at 9:08 PM, David Miller <da...@davemloft.net> wrote: >> From: Rainer Weikusat <rweiku...@mobileactivedefense.com> >> Date: Tue, 29 Dec 2015 19:42:36 + >> >>> Ja

Re: [PATCH] net: Fix potential NULL pointer dereference in __skb_try_recv_datagram

Jacob Siverskog writes: > This should fix a NULL pointer dereference I encountered (dump > below). Since __skb_unlink is called while walking, > skb_queue_walk_safe should be used. The code in question is: skb_queue_walk(queue, skb) { *last = skb;

[PATCH] af_unix: Fix splice-bind deadlock

described above). Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> Tested-by: Dmitry Vyukov <dvyu...@google.com> --- I also think this is a better (or at least more correct) solution than the pretty obvious idea to record that the socket is in the process of being bound an

splice-bind deadlock (was: [PATCH] af_unix: Revert 'lock_interruptible' in stream receive code)

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > Hannes Frederic Sowa <han...@stressinduktion.org> writes: > > [...] > >> There is still a deadlock lingering around > > [...] > >> http://lists.openwall.net/netdev/2015/11/10/

Re: [PATCH] af_unix: Revert 'lock_interruptible' in stream receive code

Hannes Frederic Sowa <han...@stressinduktion.org> writes: > On 16.12.2015 21:09, Rainer Weikusat wrote: >> With b3ca9b02b00704053a38bfe4c31dbbb9c13595d0, the AF_UNIX SOCK_STREAM >> receive code was changed from using mutex_lock(>readlock) to >> mutex_lock_inter

Re: [PATCH] af_unix: Revert 'lock_interruptible' in stream receive code

Hannes Frederic Sowa writes: [...] > There is still a deadlock lingering around [...] > http://lists.openwall.net/netdev/2015/11/10/4 Interesting problem. Assuming the description (a while ago) A: socketpair() B: splice() from a pipe to

[PATCH] af_unix: Revert 'lock_interruptible' in stream receive code

reader. As the interruptible locking makes the code more complicated in exchange for no benefit, change it back to using mutex_lock. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- Considering that the datagram receive routine also doesn't go the sleep with the mutex

[PATCH RFC] AF_UNIX SOCK_STREAM SO_PEEK_OFS oddity

issue) described above, it will actually print TWOTIMES twices followed by 12345678 while the NOTATALL remains invisible. If this is not the intended behaviour, I propose the patch below to fix it. It changes the code to reload the peek offset after the sleep. Signed-off-by: Rainer Weikusat <rweiku.

Re: [PATCH] fix inverted test in __skb_recv_datagram

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> > Date: Mon, 07 Dec 2015 23:30:58 + > >> As the kernel generally uses negated error numbers, *err needs to be >> compared with -EAGAIN (d'oh). >> &g

Re: [PATCH] fix inverted test in __skb_recv_datagram

As the kernel generally uses negated error numbers, *err needs to be compared with -EAGAIN (d'oh). Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> Fixes: ea3793ee29d3 ("core: enable more fine-grained datagram reception control") --- diff --git a/net/core/datag

Re: [PATCH] fix inverted test in __skb_recv_datagram

David Miller writes: [...] > Please, in the future, place proper subsystem prefixes in your > Subject lines. In this case, "net: " would have been appropriate > and it wouldn't be the end of the world if you capitalized > your commit header line too. > > I fixed both of

breaks blocking receive for other users (was: [PATCH 01/02] core: enable more fine-grained datagram reception control)

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> > Date: Sun, 06 Dec 2015 21:11:34 + > >> The __skb_recv_datagram routine in core/ datagram.c provides a general >> skb reception factility supposed t

[PATCH] fix inverted test in __skb_recv_datagram

As the kernel generally uses negated error numbers, *err needs to be compared with -EAGAIN (d'oh). Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> Fixes: ea3793ee29d3 --- diff --git a/net/core/datagram.c b/net/core/datagram.c index 7daff66..fa9dc64 100644 --- a/ne

[PATH 02/02] af_unix: fix unix_dgram_recvmsg entry locking

ilable regardless of any concurrent blocking readers and all blocking readers will end up sleeping via schedule_timeout, thus honouring the configured socket receive timeout. Signed-Off-By: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- diff --git a/net/unix/af_unix.c b/net/unix/af_

[PATCH 01/02] core: enable more fine-grained datagram reception control

wait_for_more_packets to __skb_wait_for_more_packets, both routines being exported interfaces. The original __skb_recv_datagram routine is reimplemented on top of these two functions such that its user-visible behaviour remains unchanged. Signed-Off-By: Rainer Weikusat <rweiku...@mobileactivedefense.com> --

Re: [RFC PATCH] af_unix: fix entry locking in unix_dgram_recvmsg

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> >> Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: >> >> [...] >> >>> Insofar I understand the comment in this code b

Re: [RFC PATCH] af_unix: fix entry locking in unix_dgram_recvmsg

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: [...] > Insofar I understand the comment in this code block correctly, > > err = mutex_lock_interruptible(>readlock); > if (unlikely(err)) { > /* recvmsg() in non b

[RFC PATCH] af_unix: fix entry locking in unix_dgram_recvmsg

help other future protocols which also need to use locks for protecting access to some per-socket state information the core/datagram.c code is unaware of. Signed-Off-By: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- Patch is against 4.4.0-rc1-net. diff --git a/include/linux/s

unix_dgram_recvmsg wakeups [test results & programs]

[to be followed by a patch] Currently, unix_dgram_recvmsg does a wake up on the peer_wait queue of a socket after every received datagram. This seems excessive, especially considering that only SOCK_DGRAM client sockets in an n:1 asssociation with a server socket ever wait for the associated

[PATCH] unix: use wq_has_sleeper in unix_dgram_recvmsg

if wq_has_sleeper indicates that someone actually wants to be notified. Testing with SOCK_SEQPACKET and SOCK_DGRAM socket seems to confirm that this is an improvment. Signed-Off-By: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c

Re: use-after-free in sock_wake_async

Eric Dumazet <eduma...@google.com> writes: > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat > <rweiku...@mobileactivedefense.com> wrote: [...] >> It's also easy to verify: Swap the unix_state_lock and >> other->sk_data_ready and see if the issue still occu

Re: use-after-free in sock_wake_async

Eric Dumazet <eric.duma...@gmail.com> writes: > On Wed, 2015-11-25 at 16:43 +0000, Rainer Weikusat wrote: >> Eric Dumazet <eduma...@google.com> writes: >> > On Tue, Nov 24, 2015 at 5:10 PM, Rainer Weikusat >> > <rweiku...@mobileactivedefense.com>

Re: use-after-free in sock_wake_async

Eric Dumazet <eric.duma...@gmail.com> writes: > On Wed, 2015-11-25 at 18:24 +0000, Rainer Weikusat wrote: >> Eric Dumazet <eric.duma...@gmail.com> writes: >> > On Wed, 2015-11-25 at 17:30 +, Rainer Weikusat wrote: >> > >> >> In case this is

Re: use-after-free in sock_wake_async

Eric Dumazet <eric.duma...@gmail.com> writes: > On Wed, 2015-11-25 at 17:30 +0000, Rainer Weikusat wrote: > >> In case this is wrong, it obviously implies that sk_sleep(sk) must not >> be used anywhere as it accesses the same struck sock, hence, when that >> can &quo

Re: use-after-free in sock_wake_async

Eric Dumazet writes: > On Wed, 2015-11-25 at 11:50 -0800, Eric Dumazet wrote: > >> > other->sk_data_ready(other); >> > + unix_state_unlock(other); > > > Also, problem with such construct is that we wakeup a thread that will > block on the lock

Re: use-after-free in sock_wake_async

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: [...] > Swap the unix_state_lock and s/lock/unlock/ :-( -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.k

Re: use-after-free in sock_wake_async

Eric Dumazet <eduma...@google.com> writes: > On Tue, Nov 24, 2015 at 3:34 PM, Rainer Weikusat > <rweiku...@mobileactivedefense.com> wrote: >> Eric Dumazet <eduma...@google.com> writes: >>> On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov

Re: use-after-free in sock_wake_async

Eric Dumazet writes: > On Tue, Nov 24, 2015 at 6:18 AM, Dmitry Vyukov wrote: >> Hello, >> >> The following program triggers use-after-free in sock_wake_async: [...] >> void *thr1(void *arg) >> { >> syscall(SYS_close, r2, 0, 0, 0, 0, 0); >>

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > David Miller <da...@davemloft.net> writes: [...] > I'm sorry for this 13th hour request/ suggestion but while thinking > about a reply to Dmitry, it occurred to me that the restart_locked/ > sk_locke

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> >> Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: >> An AF_UNIX datagram socket being the client in an n:1 association [...] > Applied a

Re: Use-after-free in ppoll

Dmitry Vyukov writes: > Hello, > > On commit f2d10565b9bdbb722bd43e6e1a759eeddb9645c8 (Nov 20). > > The following program triggers use-after-free: > > // autogenerated by syzkaller (http://github.com/google/syzkaller) > #include > #include > #include > #include > > void

Re: Use-after-free in ppoll

Dmitry Vyukov <dvyu...@google.com> writes: > On Sun, Nov 22, 2015 at 3:32 PM, Rainer Weikusat > <rweiku...@mobileactivedefense.com> wrote: >> Dmitry Vyukov <dvyu...@google.com> writes: >>> Hello, >>> >>> On commit f2d10565b9bdbb722bd43e6

Re: Use-after-free in ppoll

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: [...] > because of the close, this routine will be called with the peer_wait > wait_queue_head of the non-closed socket of the socket pair as > wait_address argument. This should have been "peer_wait wait_qu

alternate queueing mechanism (was: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue)

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: [AF_UNIX SOCK_DGRAM throughput] > It may be possible to improve this by tuning/ changing the flow > control mechanism. Out of my head, I'd suggest making the queue longer > (the default value is 10) and delaying

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

Jason Baron <jba...@akamai.com> writes: > On 11/19/2015 06:52 PM, Rainer Weikusat wrote: > > [...] > >> @@ -1590,21 +1718,35 @@ restart: >> goto out_unlock; >> } >> >> -if (unix_peer(other) != sk &&a

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: An AF_UNIX datagram socket being the client in an n:1 association with some server socket is only allowed to send messages to the server if the receive queue of this socket contains at most sk_max_ack_backlog datagrams. This i

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: > Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > > [...] > >> The basic options would be >> >> - return EAGAIN even if sending became possible (Jason's most >>

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

poll, thus avoiding the use-after-free, while still ensuring that no blocked writer sleeps forever. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> Fixes: ec0d215f9420 ("af_unix: fix 'poll for write'/connected DGRAM sockets") --- This has been created around midnigh

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> > Date: Mon, 16 Nov 2015 22:28:40 + > >> An AF_UNIX datagram socket being the client in an n:1 [...] > So because of a corner case of epoll handling and

more statistics (was: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:))

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: [...] > Some more information on this: Running the test program included below > on my 'work' system (otherwise idle, after logging in via VT with no GUI > running)/ quadcore AMD A10-5700, 3393.984 for 20 tim

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Jason Baron <jba...@akamai.com> writes: > On 11/15/2015 01:32 PM, Rainer Weikusat wrote: > >> >> That was my original idea. The problem with this is that the code >> starting after the _lock and running until the main code path unlock has >> to be execute

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: [...] > This leaves only the option of a somewhat incorrect solution and what is > or isn't acceptable in this respect is somewhat difficult to decide. The > basic options would be [...] > - retry sendin

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: [...] > The basic options would be > > - return EAGAIN even if sending became possible (Jason's most > recent suggestions) > > - retry sending a limited number of

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> > Date: Mon, 16 Nov 2015 22:28:40 + > >> An AF_UNIX datagram socket being the client in an n:1 association with >> some server socket is only allow

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue (w/ Fixes:)

poll, thus avoiding the use-after-free, while still ensuring that no blocked writer sleeps forever. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> Fixes: ec0d215f9420 ("af_unix: fix 'poll for write'/connected DGRAM sockets") --- Additional remark about &

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

poll, thus avoiding the use-after-free, while still ensuring that no blocked writer sleeps forever. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- - fix logic in _dgram_sendmsg: queue limit also needs to be enforced for unconnected sends -

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Jason Baron <jba...@akamai.com> writes: > On 11/13/2015 01:51 PM, Rainer Weikusat wrote: > > [...] > >> >> -if (unix_peer(other) != sk && unix_recvq_full(other)) { >> -if (!timeo) { >> -err

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

poll, thus avoiding the use-after-free, while still ensuring that no blocked writer sleeps forever. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- "Believed to be least buggy version" - disconnect from former peer in _dgram_connect -

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Hannes Frederic Sowa <han...@stressinduktion.org> writes: > On Wed, Nov 11, 2015, at 17:12, Rainer Weikusat wrote: >> Hannes Frederic Sowa <han...@stressinduktion.org> writes: >> > On Tue, Nov 10, 2015, at 22:55, Rainer Weikusat wrote: >> >> An AF_UNIX

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Jason Baron writes: >> + >> +/* Needs sk unix state lock. After recv_ready indicated not ready, >> + * establish peer_wait connection if still needed. >> + */ >> +static int unix_dgram_peer_wake_me(struct sock *sk, struct sock *other) >> +{ >> +int connected; >> + >> +

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Hannes Frederic Sowa <han...@stressinduktion.org> writes: > On Tue, Nov 10, 2015, at 22:55, Rainer Weikusat wrote: >> An AF_UNIX datagram socket being the client in an n:1 association with >> some server socket is only allowed to send messages to the server if the >> r

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

Jason Baron <jba...@akamai.com> writes: > On 11/09/2015 09:40 AM, Rainer Weikusat wrote: [...] >> -if (unix_peer(other) != sk && unix_recvq_full(other)) { >> +if (!unix_dgram_peer_recv_ready(sk, other)) { >> if (!timeo) { &g

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

David Miller <da...@davemloft.net> writes: > From: Rainer Weikusat <rweiku...@mobileactivedefense.com> > Date: Mon, 09 Nov 2015 14:40:48 + > >> +__remove_wait_queue(_sk(u->peer_wake.private)->peer_wait, >> +

Re: [PATCH] unix: avoid use-after-free in ep_remove_wait_queue

poll, thus avoiding the use-after-free, while still ensuring that no blocked writer sleeps forever. Signed-off-by: Rainer Weikusat <rweiku...@mobileactivedefense.com> --- - use wait_queue_t passed as argument to _relay - fix possible deadlock and logic error i

[PATCH] unix: avoid use-after-free in ep_remove_wait_queue

An AF_UNIX datagram socket being the client in an n:1 association with some server socket is only allowed to send messages to the server if the receive queue of this socket contains at most sk_max_ack_backlog datagrams. This implies that prospective writers might be forced to go to sleep despite

Re: Use-after-free in ep_remove_wait_queue

Jason Baron writes: > On 11/06/2015 08:06 AM, Dmitry Vyukov wrote: >> On Mon, Oct 12, 2015 at 2:17 PM, Dmitry Vyukov wrote: >>> On Mon, Oct 12, 2015 at 2:14 PM, Eric Dumazet >>> wrote: On Mon, 2015-10-12 at 14:02 +0200, Michal

Re: [RFC] unix: fix use-after-free in unix_dgram_poll()/ 4.2.5

Olivier Mauras writes: [...] > I've encountered issues with Jason's patch ported to 3.14.x which would break > openldap, rendering it unable to answer any query - Here's a strace of the > slapd process in this state http://pastebin.ca/3226383 > Just ported Rainer's patch to

Re: [RFC] unix: fix use-after-free in unix_dgram_poll()/ 4.2.5

Same changes ported to 4.2.5 with some minor improvments (I hope), namely, - applied a round of DeMorgan to the 'quick' check function in order to simplify the condition - fixed a (minor) error in the dgram_sendmsg change: In case the 2nd check resulted in

Re: [RFC] unix: fix use-after-free in unix_dgram_poll()

Jason Baron <jba...@akamai.com> writes: > On 10/28/2015 12:46 PM, Rainer Weikusat wrote: >> Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: >>> Jason Baron <jba...@akamai.com> writes: [...] >> and the not-so-nice additional p

[RFC] unix: fix use-after-free in unix_dgram_poll()

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > Jason Baron <jba...@akamai.com> writes: [...] >> 2) >> >> For the case of epoll() in edge triggered mode we need to ensure that >> when we return -EAGAIN from unix_dgram_sendmsg() whe

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Rainer Weikusat <r...@doppelsaurus.mobileactivedefense.com> writes: > Jason Baron <jba...@akamai.com> writes: >> On 10/18/2015 04:58 PM, Rainer Weikusat wrote: [...] >> 1) >> >> In unix_peer_wake_relay() function, 'sk_wq' is an __rcu pointer and t

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Jason Baron <jba...@akamai.com> writes: > On 10/18/2015 04:58 PM, Rainer Weikusat wrote: > > [...] > >> >> The idea behind 'the wait queue' (insofar I'm aware of it) is that it >> will be used as list of threads who need to be notified when the >> associ

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Jason Baron <jba...@akamai.com> writes: > >> >> X-Signed-Off-By: Rainer Weikusat <rweiku...@mobileactivedefense.com> >> Sorry for the delayed reply but I had to do some catching up on what the people who pay me consider 'useful work' :-). > So the pa

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Jason Baron <jba...@akamai.com> writes: > On 10/12/2015 04:41 PM, Rainer Weikusat wrote: >> Jason Baron <jba...@akamai.com> writes: >>> On 10/05/2015 12:31 PM, Rainer Weikusat wrote: [...] >> OTOH, something I seriously dislike about your relaying im

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

David Miller writes: > From: Jason Baron > 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

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Jason Baron <jba...@akamai.com> writes: > On 10/05/2015 12:31 PM, Rainer Weikusat wrote: [...] >> Here's a more simple idea which _might_ work. The underlying problem >> seems to be that the second sock_poll_wait introduces a covert reference >> to the peer s

Re: [PATCH v4 1/3] net: unix: fix use-after-free in unix_dgram_poll()

Hannes Frederic Sowa writes: > Jason Baron writes: > >> The unix_dgram_poll() routine calls sock_poll_wait() not only for the wait >> queue associated with the socket s that we are poll'ing against, but also >> calls >> sock_poll_wait() for a

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Eric Dumazet <eric.duma...@gmail.com> writes: > On Mon, 2015-10-05 at 17:31 +0100, Rainer Weikusat wrote: > >> atomic_long_set(>inflight, 0); >> INIT_LIST_HEAD(>link); >> @@ -2135,8 +2139,16 @@ static unsigned int unix_poll(struct fil >> stati

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Rainer Weikusat <rweiku...@mobileactivedefense.com> writes: > Mathias Krause <mini...@googlemail.com> writes: >> On 2 October 2015 at 22:43, Jason Baron <jba...@akamai.com> wrote: >>> The unix_dgram_poll() routine calls sock_poll_wait() not only for the wait &g

Re: [PATCH v2 1/3] unix: fix use-after-free in unix_dgram_poll()

Mathias Krause writes: > On 2 October 2015 at 22:43, Jason Baron wrote: >> The unix_dgram_poll() routine calls sock_poll_wait() not only for the wait >> queue associated with the socket s that we are poll'ing against, but also >> calls [useless

  1   2   >